Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing PBC conv_args (draft) #299

Closed
wants to merge 1 commit into from

Conversation

RylieWeaver
Copy link
Collaborator

@RylieWeaver RylieWeaver commented Oct 29, 2024

This PR aims to address the issue related to generating conv_args that are in-line with periodic boundary conditions.

The following implementation should be robust under the following assumptions, which I think should always be true:
(1) radius < min(supercell_size) / 2
(2) The edge_index has been generated correctly with periodic boundary conditions
(3) data.supercell_size exists if and only if pbc conditions are used

Currently, only two models are updated to use the new functionality. If this is a good solution, it can be easily extended to the others.

@RylieWeaver
Copy link
Collaborator Author

RylieWeaver commented Oct 29, 2024

@allaffa This should give an idea of the implementation I've been working with. I can still take a further look at the Facebook implementation you mentioned, but since this one is already working and I'm familiar with I wanted to show you what it would look like first. Let me know your thoughts.

@RylieWeaver RylieWeaver requested a review from allaffa October 29, 2024 02:04
@RylieWeaver RylieWeaver marked this pull request as draft October 29, 2024 02:11
@RylieWeaver RylieWeaver changed the title fixing pbc conv args draft Fixing PBC conv_args (draft) Oct 29, 2024
@RylieWeaver RylieWeaver closed this Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant