-
Notifications
You must be signed in to change notification settings - Fork 86
Fixes and improvements for CHARMM force fields #386
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
Conversation
…e atom type scheme.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #386 +/- ##
=======================================
Coverage 71.41% 71.41%
=======================================
Files 5 5
Lines 822 822
=======================================
Hits 587 587
Misses 235 235 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I looked over the changes, but I don't think I know enough to review it properly. I'll trust you on it. Is there an open issue describing the problems this fixes? I assume we'll want to create a 8.3.1 patch with the updated force fields? |
|
Yes, I should have linked back to openmm/openmm#4976. Unfortunately the new CHARMM we released fails to parameterize systems with the existing water models that we didn't update (it raises an error instead), and the test in OpenMM that was loading the new force field doesn't use a system that has water in it, so I didn't catch it. My plan after getting this merged into openmmforcefields was to open a PR for OpenMM that updates the force fields and adds a test that verifies that they work with the water models in OpenMM. We should probably do a patch because the CHARMM36 2024 released now only works with the water models in openmmforcefields. |
|
@peastman If this looks OK, can you approve? Although I have write access to this repository, the main branch is protected by a policy that doesn't let me merge unless another user explicitly approves the review. I want to make sure this can be merged in to openmmforcefields and the main branch passes CI before openmm/openmm#4989 is merged. |
convert_charmm.py: previously, improper/anisotropy assignment scripts would raise errors due to inability to parse the atom names