Codecov Report

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

❗ 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     

... 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.

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

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

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.

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 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

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.