Implement AddPositionalEncoding transform

[dongkwan-kim]

This PR includes implementations of Laplacian eigenvector positional encoding & random walk positional encoding based on the original authors' implementation.

• https://github.com/graphdeeplearning/benchmarking-gnns/blob/ef8bd8c7d2c87948bc1bdd44099a52036e715cd0/data/COLLAB.py#L12
• https://github.com/vijaydwivedi75/gnn-lspe/blob/21d2a654569fcac42cde28ce0695386d1c56a9b9/data/ogb_mol.py#L97

Any comments are welcome. Plus, it looks fine to me but I do not know why there is an indentation error in the documentation build...

[codecov]

Codecov Report

Merging #4521 (161e793) into master (8fdf895) will increase coverage by 0.06%.
The diff coverage is 98.55%.

@@            Coverage Diff             @@
##           master    #4521      +/-   ##
==========================================
+ Coverage   82.81%   82.88%   +0.06%     
==========================================
  Files         315      316       +1     
  Lines       16605    16674      +69     
==========================================
+ Hits        13752    13820      +68     
- Misses       2853     2854       +1     

Impacted Files	Coverage Δ
torch_geometric/utils/__init__.py	100.00% <ø> (ø)
...ch_geometric/transforms/add_positional_encoding.py	98.27% <98.27%> (ø)
torch_geometric/transforms/__init__.py	100.00% <100.00%> (ø)
torch_geometric/utils/loop.py	84.78% <100.00%> (+1.85%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fdf895...161e793. Read the comment docs.

[Padarn]

I guess the paper is this one: GRAPH NEURAL NETWORKS WITH LEARNABLE
STRUCTURAL AND POSITIONAL REPRESENTATIONS? Perhaps we can add a reference to it in the doc?

[dongkwan-kim]

I guess the paper is this one: GRAPH NEURAL NETWORKS WITH LEARNABLE STRUCTURAL AND POSITIONAL REPRESENTATIONS? Perhaps we can add a reference to it in the doc?

@Padarn I added the paper you mentioned in the docs of lines 32 -- 34.

pytorch_geometric/torch_geometric/transforms/add_positional_encoding.py

Lines 32 to 34 in 9f52a53

	positional encoding from the `"Graph Neural Networks with Learnable
	Structural and Positional Representations"
	<https://arxiv.org/abs/2110.07875>`_ paper.

Is this what you mean, or if there is anything I missed, could you specify it? Thank you.

[Padarn]

Yes thats all I meant, just common practice across the docstrings, thanks!

[rusty1s]

This looks pretty good. Thank you very much! Only have some minor points regarding code structure.

[rusty1s]

Any chance we can also test for output rather than solely checking for shapes?

[dongkwan-kim]

I added the output tests for RandomWalkPE.
However, for LaplacianEigenvectorPE, I found that each run produces different outputs. I guess computing eigenvectors of Laplacian is not a deterministic operation. How can we write output tests for LaplacianEigenvectorPE? Any suggestion?

[Padarn]

You can try setting the random seed of torch and numpy?

[dongkwan-kim]

I tried np.random.seed and random.seed, but it still creates different outputs. I found that using a fixed value for v0 in scipy.linalg.eigs produces deterministic outputs. We can place **kwargs in __call__ and pass the kwargs (including v0) into eigs explicitly. What do you think?

[Padarn]

Yeah, it looks like when it's not provided there is a flag 'info=0' set.

How different are the outputs, personally I feel maybe adding some leniency in the checking of the result (i.e using all_close with a high atol pr rtol) might be better than adding the extra **kwargs just for this.

Just my opinion though. I think both ways work.

[dongkwan-kim]

I have tested various graphs for AddLaplacianEigenvectorPE. If there is no v0, the output really varies so we cannot test with torch.allclose.

Instead, I added a clustering test on a graph with two clusters, which uses the mean value of the first non-trivial eigenvector (Fiedler vector) of each cluster.

Also, I added tests with exact values using seed_everything and v0. The keyword argument v0 will be added through AddLaplacianEigenvectorPE.__init__.

[dongkwan-kim]

Well... But looking at failed tests, using seed_everything and v0 does not guarantee the same output in different machines.

[Padarn]

Got it. I think what you've done for the clustering test is a good solution. Thanks!

[dongkwan-kim]

This is super great. Thank you all. I will send other PRs mentioned in this thread soon.

[rusty1s]

Thank you!

[Padarn]

Great!