search

learningmatter-mit / NeuralForceField

Neural Network Force Field based on PyTorch
MIT License
237 stars 50 forks source link

GraphAttention.message only uses ij contributions -- never ji -- to form message #6

Closed rsokl closed 2 years ago

rsokl commented 2 years ago

a_ji is not incorporated into the actual message that gets returned by GraphAttention.message:

https://github.com/learningmatter-mit/NeuralForceField/blob/f06f1a1aae2bb95a92cdf7657863e5f6a21bd983/nff/nn/modules/schnet.py#L293-L305

simonaxelrod commented 2 years ago

Good find! This commit fixes it. The graph attention code was written a while back and I don't think we've used it any of our models. So I would be extra careful with it, and also with anything that isn't referenced in a tutorial (and hence thoroughly tested)

rsokl commented 2 years ago

Thanks for the tip 😃 ! I just came across it by chance as I was going through some of the source code -- my IDE highlighted the unused variable. I'm not using that particular code.

simonaxelrod commented 2 years ago

Awesome, thanks for letting us know! Gotta love those IDEs

On Wed, Feb 2, 2022 at 12:59 PM Ryan Soklaski @.***> wrote:

Thanks for the tip 😃 ! I just came across it by chance as I was going through some of the source code -- my IDE highlighted the unused variable. I'm not using that particular code.

— Reply to this email directly, view it on GitHub https://github.com/learningmatter-mit/NeuralForceField/issues/6#issuecomment-1028206031, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHTEAXNXCRY2NI4H3BGUAZ3UZFWG5ANCNFSM5NKEMVYQ . You are receiving this because you modified the open/close state.Message ID: @.***>