Adding Graphormer layer

CarloLucibello / GraphNeuralNetworks.jl

Graph Neural Networks in Julia

https://carlolucibello.github.io/GraphNeuralNetworks.jl/dev/

MIT License

214 stars 47 forks source link

Adding Graphormer layer #275

Closed 5hv5hvnk closed 1 year ago

5hv5hvnk commented 1 year ago

Continued from #265 fixes #251 To do:

[ ] struct based layers
[ ] doc-string
[ ] example pipeline
[ ] unit tests
[ ] pass build tests
[ ] fixed conflicts

codecov[bot] commented 1 year ago

Codecov Report

Merging #275 (99813a6) into master (05fca7c) will decrease coverage by 1.62%. The diff coverage is 0.00%.

:exclamation: Current head 99813a6 differs from pull request most recent head 7387752. Consider uploading reports for the commit 7387752 to get more accurate results

@@            Coverage Diff             @@
##           master     #275      +/-   ##
==========================================
- Coverage   82.36%   80.74%   -1.62%     
==========================================
  Files          17       19       +2     
  Lines        1911     1958      +47     
==========================================
+ Hits         1574     1581       +7     
- Misses        337      377      +40

Impacted Files	Coverage Δ
src/layers/conv.jl	`74.65% <0.00%> (-4.61%)`	:arrow_down:

... and 11 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

CarloLucibello commented 1 year ago

This is not capturing the essential features of the paper:

The message function is not defined. Since this layer is basically a transformer, the attention runs on the whole graph instead of just the neighborhood of a node. Therefore, we shouldn't use the usual message passing scheme but we can rely on the MultiHeadAttention layer from Flux (soon to be released).
The bias term of eq. 5 is missing. The phi function should be given at construction time
As part of the layer, eq. 8 and eq. 9 prescribe layer norm, residual connection, and an MLP.
edge features can be considered in a second PR, and should implement the mechanics of eq. 7

CarloLucibello commented 1 year ago

Why you checked the tests as done? I see no tests...

5hv5hvnk commented 1 year ago

Why you checked the tests as done? I see no tests...

I have written some tests you can check here I don't know why It isn't showing in the files changed let me look into it.

5hv5hvnk commented 1 year ago

I have written some tests you can check here I don't know why It isn't showing in the files changed let me look into it.

@CarloLucibello Hi can you look into the updates and see the tests?

CarloLucibello commented 1 year ago

@CarloLucibello Hi can you look into the updates and see the tests?

can you make the tests part of this PR? it's weird they are somewhere else

5hv5hvnk commented 1 year ago

Hello @CarloLucibello Sorry for delayed response on this I believe we still need this layer, I redone each line of code after reading through the paper and more Julia blogs (mostly linked to flux) to have better understanding.