search

ORNL / HydraGNN

Distributed PyTorch implementation of multi-headed graph convolutional neural networks
BSD 3-Clause "New" or "Revised" License
54 stars 24 forks source link

Open catalyst 2022 #214

Closed allaffa closed 3 months ago

allaffa commented 3 months ago

This PR include the following:

  1. removes unused import in Open Catalyst 2020 example
  2. includes new example for Open Catalyst 2022. I tested the code on Frontier to create AIOS binary files and kick off the training, and it works.
allaffa commented 3 months ago

Looks good to me overall. One comment is that in the current implementation, using the relative node position as an edge feature does not seem to achieve rotation equivariance.

@pzhanggit Goo point. In preparation for the GB work, I do not know whether it is best to just use bond length and use equivariance, or renounce to equivariance and add more informative edge features. Honestly, I do not even think we will have time to test all the possible combinations of input parameters to see what works best. What do you suggest?

pzhanggit commented 3 months ago

Looks good to me overall. One comment is that in the current implementation, using the relative node position as an edge feature does not seem to achieve rotation equivariance.

@pzhanggit Goo point. In preparation for the GB work, I do not know whether it is best to just use bond length and use equivariance, or renounce to equivariance and add more informative edge features. Honestly, I do not even think we will have time to test all the possible combinations of input parameters to see what works best. What do you suggest?

Given the short timeline, I suggest we remove the relative position from edge feature to keep the equivariance.

allaffa commented 3 months ago

Looks good to me overall. One comment is that in the current implementation, using the relative node position as an edge feature does not seem to achieve rotation equivariance.

@pzhanggit Goo point. In preparation for the GB work, I do not know whether it is best to just use bond length and use equivariance, or renounce to equivariance and add more informative edge features. Honestly, I do not even think we will have time to test all the possible combinations of input parameters to see what works best. What do you suggest?

Given the short timeline, I suggest we remove the relative position from edge feature to keep the equivariance.

@pzhanggit I just updated both Open Catalyst 2020 and Open Catalyst 2022 examples so that only the distance between nodes is used as edge feature.