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

Support for OpenMM #4

Open
wants to merge 171 commits into
base: htmd
Choose a base branch
from
Open

Conversation

orionarcher
Copy link

@orionarcher orionarcher commented Nov 11, 2021

Summary

Include a summary of major changes in bullet points:

Additional dependencies introduced (if any)

openmm: this is code for OpenMM IO

openff: needed to parameterize Sage Force Field

parmed: used to create suitably sized PDB files. Later may be used to add more force fields.

openbabel + rdkit are used, but they are already used in pymatgen.

TODO (if any)

  • better support for water models, at least allow the user to specify TIP3 or TIP4.

  • automatic partial charge assignment. It should be possible to include RESP partial charges in the parameterization.

  • allow partial charge scaling as a setting in the generator

Checklist

  • Code is in the standard Python style. The easiest way to handle this
    is to run the following in the correct sequence on your local machine. Start with running
    black on your new code. This will automatically reformat
    your code to PEP8 conventions and removes most issues. Then run
    pycodestyle, followed by
    flake8.
  • Docstrings have been added in the Google docstring format.
    Run pydocstyle on your code.
  • Type annotations are highly encouraged. Run mypy to type check your code.
  • Tests have been added for any new functionality or bug fixes.
  • All linting and tests pass.

@rkingsbury
Copy link
Owner

Thanks @orioncohen ! I won't be able to look at this in much detail until early next week but I'm excited to see how your OpenMM support is shaping up.

@orionarcher orionarcher changed the title [WIP [WIP] support for OpenMM Nov 11, 2021
@orionarcher
Copy link
Author

Sounds good!

I believe that the OpenMM IO can fit into the new IO abstraction pretty cleanly by serializing all of the OpenMM objects needed to create a simulation

The InputSetGenerator will be instantiated by user defined settings and create a Topology, System, Integrator, and Simulation. It will then serialize all of those objects into XML and store them. When get_input_set is called, it will create an InputSet that has the XML files as attributes. This process matches the rest of Pymatgen IO and is fit for HTMD.

The InputSet can then have a get_simulation method that instantiates a simulation and returns it. This way, no OpenMM objects are ever stored and the InputSetGenerator and InputSet remain MSONable.

@orionarcher
Copy link
Author

What should determine if a parameter goes in InputGenerator.__init__ or InputGenerator.get_input_set?

The documentation seems to indicate that only the structure and other critical parameters should go in get_input_set, but I can imagine many situations where that would be inconvenient. For example, if I wanted to repeat a simulation at four different temperatures. I'm tempted to put all simulations settings in both __init__ and get_input_set for maximum flexibility, but I'm curious to hear more about the intended design.

Alex Epstein and others added 28 commits January 21, 2022 09:17
deleted print statement from debug
Added periodic boundaries to OpenMM System. Assumed box is in angstroms and manually converted to nanometers because not sure how units work
Improvements on forcefield specification for mixed systems and specific water models
Implemented changes in comments, ran pre-commit
Did my best to address all comments
Added function to assign_biopolymer_and_water_ff
Adding unit tests for:
1. smiles with formal charge
2. Ensuring different water models assign different parameters
3. Testing mixing of small molecule ff with water ff

Also removed tip4p support because it requires the addition of an extra particle. Need to figure that out
Fixed smiles string in test_formal_charge
Be careful about smiles!!
ran black
Support for multiple forcefields
janosh pushed a commit to manaslkht/pymatgen that referenced this pull request Oct 22, 2022
updates:
- [github.com/PyCQA/autoflake: v1.4 → v1.6.1](PyCQA/autoflake@v1.4...v1.6.1)
- [github.com/asottile/pyupgrade: v2.37.3 → v2.38.2](asottile/pyupgrade@v2.37.3...v2.38.2)
- [github.com/psf/black: 22.6.0 → 22.8.0](psf/black@22.6.0...22.8.0)
- [github.com/PyCQA/flake8: 5.0.2 → 5.0.4](PyCQA/flake8@5.0.2...5.0.4)
- [github.com/pre-commit/mirrors-mypy: v0.971 → v0.981](pre-commit/mirrors-mypy@v0.971...v0.981)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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.

2 participants