-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add support for hoomd.md.improper.Periodic
in to_hoomd_forcefield
#807
Conversation
So, there is kind of a band-aid fix in here that I think is sort of related to #767, but when the forcefield uses wild cards, these carry over into Right now, the snapshot writer uses The band-aid fix is to check for IMO, I think this is fine for now, but there is still the overall larger issue of having more flexibility in both giving instructions on what to use in the writers (types vs classes) as well as flexibility in combining them (as is discussed in #767) |
This should be ready to merge, the only test failing is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets get #815 merged in to main and to this PR, then I think it's good to go.
Raise and issue about python < 3.12 though once this is merged, since we'll want to fix that when we get to that issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This PR starts adding support for periodic improper potentials as they were recently added to Hoomd version 4.5.
I'm running into some issues, I figured I'd get the PR submitted so there are some other eyes on it as well. Here is a gist of the notebook I'm testing it out with..
One note: I made some changes to the environment file because I was having issues with mBuild plugins using mBuild < 17. This is a separate issue, but the pinned versions in
environment-dev.yml
are changed here to fix that for now.The contents of the notebook is basically the unit test we have for using GAFF, but with benzene instead of ethanol.
It looks like hoomd improper force params are being written with the wild cards?
This doesn't match the snapshot improper types
I'll keep working on this, but wanted to get it out there for review; maybe I'm missing something obvious at the moment.