search

ORNL / HydraGNN

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

Incorporating Equivariance #194

Closed JustinBakerMath closed 11 months ago

JustinBakerMath commented 11 months ago

A reformatting of the Base class and the inherited classes to support equivariance through updating positional data (data.pos) in each graph convolution.

allaffa commented 11 months ago

@JustinBakerMath we need to resolve some conflicts with the new version of the branch since I merged PR#195 (activation functions). Let me know if you want me to take care of the rebasing myself. Hopefully, this will be pretty straightforward to take care of.

jychoi-hpc commented 11 months ago

I was able to run a few examples without errors. But, I am not sure if I really tested this new equivariance feature. I am just wondering if there is any example in the list of our examples to check this new feature. I am thinking simply turning on "equivariance: true" and changing "model_type: EGNN or SchNet".

Otherwise, it looks good. I ran unit tests (no mpi) and they went ok in my local machine.

allaffa commented 11 months ago

I was able to run a few examples without errors. But, I am not sure if I really tested this new equivariance feature. I am just wondering if there is any example in the list of our examples to check this new feature. I am thinking simply turning on "equivariance: true" and changing "model_type: EGNN or SchNet".

Otherwise, it looks good. I ran unit tests (no mpi) and they went ok in my local machine.

@jychoi-hpc
yes, we can use the equivariant for the QM9 dataset.

JustinBakerMath commented 11 months ago

@JustinBakerMath we need to resolve some conflicts with the new version of the branch since I merged PR#195 (activation functions). Let me know if you want me to take care of the rebasing myself. Hopefully, this will be pretty straightforward to take care of.

I have now merged the changes to the activation functions to the changes for equivariance.

allaffa commented 11 months ago

@JustinBakerMath we need to resolve some conflicts with the new version of the branch since I merged PR#195 (activation functions). Let me know if you want me to take care of the rebasing myself. Hopefully, this will be pretty straightforward to take care of.

I have now merged the changes to the activation functions to the changes for equivariance.

@JustinBakerMath Thanks. I will run the equivariance with the QM9 dataset. @jychoi-hpc would you mind doing the same? The best example for this would be the GDB-9-Ex dataset. However, the GDB-9-Ex dataset is not updated in the current version of he main branch. So I think that using the standard QM9 is the best thing to do.

allaffa commented 11 months ago

Looks good to me, thanks!

@pzhanggit, thanks! I just asked @JustinBakerMath to move unsorted_segment_mean into hydragnn/utils/models.py.

allaffa commented 11 months ago

I was able to run a few examples without errors. But, I am not sure if I really tested this new equivariance feature. I am just wondering if there is any example in the list of our examples to check this new feature. I am thinking simply turning on "equivariance: true" and changing "model_type: EGNN or SchNet". Otherwise, it looks good. I ran unit tests (no mpi) and they went ok in my local machine.

@jychoi-hpc yes, we can use the equivariant for the QM9 dataset.

@jychoi-hpc I ran the QM9 example in debug mode using PyCharm with model_type = "EGNN" and equivariance = True. I could see the equivariant message passing being activated.

jychoi-hpc commented 11 months ago

Great. I was able to run too with model_type = "EGNN" and equivariance = True for QM9. This PR looks good to me.