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

Fix a bug the periodic boundary conditions in ANI SymmetryFunctions #83

Merged
merged 1 commit into from
Feb 28, 2023

Conversation

sef43
Copy link
Contributor

@sef43 sef43 commented Feb 27, 2023

This aims to fix #82
It sets the periodic flag in SymmetryFunctions.cpp based on if the cell vectors are defined as suggested by @peastman

I have added a new test case that is a water box with PBCs, this is run in the TestOptimizedTorchANI.py and TestSymmetryFunctions.py tests where the energy and forces of the NNPOps implementation are compared to the TorchANI implementation.

periofic boundary flag correctly set in SymmetryFunctions
added tests with a water box with PBCs
@peastman
Copy link
Member

Looks good to me. All tests have passed. I'll leave it to @raimis to merge if it looks ok to him.

@raimis raimis self-requested a review February 28, 2023 14:35
@raimis raimis merged commit 16543f9 into openmm:master Feb 28, 2023
@raimis
Copy link
Contributor

raimis commented Feb 28, 2023

@sef43 thanks for the fix!

@raimis raimis mentioned this pull request Mar 2, 2023
@raimis raimis changed the title Fix PBCs Fix a bug the periodic boundary conditions in ANI SymmetryFunctions Mar 3, 2023
@raimis
Copy link
Contributor

raimis commented Mar 3, 2023

Rename to make a release log clear.

@sef43 sef43 deleted the fix_pbc branch March 7, 2023 16:59
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.

Surprising differences between nnpops and torchani
3 participants