forked from materialsproject/pymatgen
-
Notifications
You must be signed in to change notification settings - Fork 1
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
Closed
Closed
Changes from all commits
Commits
Show all changes
171 commits
Select commit
Hold shift + click to select a range
29d6678
initial files for Pymatgen OpenMM IO
orionarcher 6ed67c1
added several convenience functions for setting up OpenMM simulations
orionarcher 052d0bb
rename files
orionarcher 80ade99
added an equilibrate pressure and anneal simulation stages
orionarcher e715cfa
added code to return a simulation object from simulation generator
orionarcher c9350a5
improved documentation for the setup classes
orionarcher 3c9a3b6
added testing files
orionarcher f1140f7
skeleton for testing setup and simulations
orionarcher 0a1c7a7
basic type checking testing
orionarcher da54ceb
fixed an issue where quotation marks inside strings were causing pack…
orionarcher f3d33b6
fixing broken tests
orionarcher e6cf399
small formatting changes to setup
orionarcher 39bdb80
bug fixes and finished basic testing for openmm setup
orionarcher b64d4b7
bug fixes and testing for the anneal simulation runner
orionarcher eb7d39a
finished bug fixes on simulation code and finished basic testing
orionarcher b4b233c
noted bug to be fixed later
orionarcher 87643d5
started adapting the openmm IO to fit the new IO abstraction
orionarcher 3998d67
corrected format of InputFiles and updated the OpenMM input set
orionarcher 9055bc2
added get_simulation and validate methods to the OpenMMSet
orionarcher dff9cb7
small bug fix to setup
orionarcher 9024ad6
update signature of OpenMMGenerator and begin migrating setup methods
orionarcher c0c8c68
added setup methods to OpenMMGenerator class
orionarcher e81713d
move from 'generate' for method names to 'get'
orionarcher 9823bbd
finish basic system parameterization for Sage force field
orionarcher db46463
add a reminder to insert partial charge handling
orionarcher 077ac74
added comments to document development plan
orionarcher 1bc612a
fixed several typos in code
orionarcher 8214179
add file for testing_inputs
orionarcher 9c77d67
updated documentation and OpenMMGenerator __init__ and get_input_set
orionarcher ad16554
update InputFile approach to remove dynamic OpenMM objects, factor in…
orionarcher dd516bb
add testing skeleton and minor syntax changes
orionarcher 9826ee2
update documentation
orionarcher 024b8d2
add testing for OpenMMGenerator
orionarcher 36985c3
add testing for static methods
orionarcher 7eb2be2
add force field file
orionarcher 359b327
adding and debugging testing
orionarcher 7f3d3dd
fixed a bug in the packmol IO
orionarcher 23f05e2
completed first set of testing for OpenMMGenerator
orionarcher dcd9c36
added abstract XmlInput class to simplify InputFile definitions
orionarcher ffcb529
rewrote input set to better fit new IO abstraction
orionarcher 3b032aa
change naming conventions for InputFiles and attempt to finish get_si…
orionarcher 4ac4fa4
added several test files and datafiles.py to organize test files
orionarcher ee14cd9
add corrected topology.pdg
orionarcher 98626c2
fixed pdb file bug and removed need for coordinates by defaulting to …
orionarcher e067bd0
finish bug fix for TopologyInput
orionarcher 21a5999
clean up
orionarcher 3f5bd5c
more clean up
orionarcher ee8eb82
fix issue with input file syntax using XmlInput
orionarcher 399f678
complete testing for InputFiles
orionarcher 16344db
added InputSet testing and completed bug fixes. Testing is now complete
orionarcher f07f818
another packmol bugfix
orionarcher 16caac5
improved documentation
orionarcher 39eb02c
finished first pass documentation
orionarcher 8be9fb8
remove unused setup.py
orionarcher 5e85188
delete testing for setup.py
orionarcher 64e1d3d
added files for testing partial charge assignment
orionarcher 684af65
added skeleton for partial charge assignment and testing
orionarcher 36f0c83
rename a directory and propagate changes
orionarcher 0515670
restructure datafiles.py and test_inputs.py to remove all hard paths
orionarcher f7f37b3
finish restructuring datafiles and begin testing automated partial ch…
orionarcher f25e374
added several files to test chirality matching with partial charge as…
orionarcher 90f1fba
deleted several useless files that I accidentally committed
orionarcher 6062609
a initial implementation of partial charge matching
orionarcher 31bcca6
rewrote get_atom_map to progressively relax stererochemistry and bond…
orionarcher 41b06d1
added testing for infer_openff_mol and get_atom_map
orionarcher f34f210
add testing for _assign_charges_to_openff_mol
orionarcher b94cebc
finished testing for partial charge inferrer
orionarcher 5e991ae
add add_partial_charges_to_forcefield, combining other partial charge…
orionarcher 994634e
remove equal length assertion in get_atom_map and rename get_box
orionarcher e5597f3
finish all testing
orionarcher 82af566
a couple small bug fixes
orionarcher a3939ea
a monstrous test that is trying to track down an enigmatic bug, will …
orionarcher 5ac6432
reorganize imports
orionarcher 99fe1b5
clean up imports and document imports
orionarcher dbb8392
finished tracking down a truly heinous bug, atom ordering was reverse…
orionarcher 6038955
removed unused files and correct error in FEC.npy test file
orionarcher c070056
small syntax and formatting changes
orionarcher bc6d0ad
Merge branch 'htmd' into htmd_openmm
rkingsbury 0b06b54
update testing to test from_file reconstruction
orionarcher cab1f6c
removed force field .xml, not currently needed
orionarcher 74c08cf
Merge remote-tracking branch 'rkingsbury/htmd' into htmd_openmm
orionarcher 23e33bf
Merge branch 'htmd_openmm' of github.com:orioncohen/pymatgen into htm…
orionarcher 664fca8
changed partial charge logic to support partial charge scaling
orionarcher 461bd63
added testing for partial charge scaling
orionarcher 98db5b7
format all files with black and run through precommit linter
orionarcher aaf9e82
modified inputs to satisfy mypy static type checking
orionarcher 587f38d
ignore type checking for pkg_resources
orionarcher 26e1005
add docstrings to all InputFile objects
orionarcher 9cf41d6
add docstring to init
orionarcher 08ec3a2
update requirements-optional file with new dependencies
orionarcher 3c876fa
updated github workflow to include openmm dependencies
orionarcher 14f9e13
update requirements-optional again
orionarcher 822f193
editing requirements-optional
orionarcher 6ff2d70
more changes to test.yml
orionarcher d45fc77
fixed error in test.yml
orionarcher 81f64d1
removed openff import from requirements-optional
orionarcher 78f4411
more testing workflow changes
orionarcher 31afad0
please pass
orionarcher c43804f
only conda dependencies in test.yml
orionarcher 3fc22ae
add stricter versioning of openmm
orionarcher ccbbec0
change openmm version to 7.6.2
orionarcher d6980a9
I think I already tried this but ¯\_(ツ)_/¯
orionarcher 42dbb1a
split InputFiles, InputSet, and InputGenerator across different files
orionarcher 9cd2176
refactor testing into multiple files
orionarcher 162b111
update simulation imports and testing
orionarcher d272547
modify packmol to fit base branch and satisfy linter
orionarcher 49f3a2a
black simulations and add kwargs to equilibrate pressure
orionarcher e3adad0
added test for write_inputs
orionarcher a2aeb3a
fix simulations functions and add real testing
orionarcher 02496b8
syntax change and removed randomness from testing by setting fixed pa…
orionarcher 3c6ecbf
change header in imports
orionarcher 1f18517
small docstring changes
orionarcher 7a97bf0
deleted __init__.py in tests
orionarcher 479735b
change path naming conventions to pass remote testing
orionarcher 42948e1
add platform and platformProperties arguments to get_simulation
orionarcher 86389a2
add new testing files for Li
orionarcher a67e001
bug fixes and code restructuring for generators.py
orionarcher 0f4afc5
added test for partial charge scaling and setting on Li/PF6 system
orionarcher 0760e38
small typo and changing args to kwargs to attempt to fix parallelizat…
orionarcher e6e30d8
another addition intended to help fix a pernicious multiprocessing error
orionarcher 9946e50
fix typing error in get_simulation docs
orionarcher f7d7ac5
added statements for debugging and changed platform syntax
orionarcher 50d556a
revert previous commit
orionarcher 685ed7d
added del statement to delete context
orionarcher 8e37b15
attempt to force garbage collection to remove Context reference
orionarcher 6b30d6b
revert del and garbage collection changes, they did not solve the issue
orionarcher ae0d670
moved several static methods from OpenMMSolutionGenerator to utils.py
orionarcher 8a32263
removed several static methods from OpenMMSolutionGen
orionarcher 608bb4d
Merge remote-tracking branch 'orioncohen/htmd_openmm' into ae_htmd
003da4c
added utility functions for molarity, volume ratio, and mass ratio ca…
orionarcher 358a794
fixed typing error
orionarcher 3969040
Merge remote-tracking branch 'orioncohen/htmd_openmm' into ae_htmd
19c20e5
changed generators to allow smiles with character
orionarcher de1c2cf
Hasty edits to make things work
a068073
Merge remote-tracking branch 'orioncohen/htmd_openmm' into ae_htmd
480ae7c
added partial_charge_method to OpenMMSolutionGen
orionarcher c6f1cc2
refactor code to assign partial charges with molecules, rather than a…
orionarcher 46fae4f
refactor testing for new partial charge method
orionarcher 75c14c1
clean up function signatures
orionarcher ed2e232
add option to specify geometries of initial molecule positions, forma…
orionarcher 10c8722
add testing for coordinate specification logic
orionarcher 61b0ad9
Mixed ForceField Systems
8a1b847
Merge remote-tracking branch 'origin/ae_htmd' into ae_htmd
3ade32e
proofreading
9fadc1d
Fixing tests
96e6964
TODO reminder for needed test
orionarcher eb7ac35
fixed bug: test_get_box now fails with incorrect output
orionarcher 9dfddf7
small formatting change
orionarcher 8111430
Alex changes
orionarcher 8d0620e
added new files for testing and deleted unused files
orionarcher a010ce8
modified datafiles to include new test files
orionarcher d2c5e59
added support for more file types to generators initial_geometries pr…
orionarcher edfaeab
added more robust testing for initial_geometries feature
orionarcher cbfc399
add a TODO reminder
orionarcher 5569e82
Merge remote-tracking branch 'rkingsbury/htmd' into htmd_openmm
orionarcher 3285239
move get_atom_map and infer_openff_mol to utils
orionarcher 7113d8e
move order_molecule_like_smiles from generators to utils
orionarcher 6c82b79
move get_coordinates from generators to utils
orionarcher 298e7f8
move get_topology from generators to utils
orionarcher 038f56b
move assign_charges_to_mols and add_mol_charges_to_forcefield from ge…
orionarcher 32fb2c6
move parameterize_system from generators to utils
orionarcher 29b4927
Support for multiple forcefields
ee3932e
pre-commit run
a6c9226
Addressing Comments
b003ff1
Separating functions
e4dd9fc
Adding coveralls
be6b48e
Trying to get linting
08d53ce
Unit tests + tip4p removal
2754454
pytest fixing
42d0475
black formatting
01efafd
Merge pull request #2 from arepstein/ae_htmd
orionarcher File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
repo_token: KpgnfDK8BDSSTSS4lSrCvHwHnchLgIQ0D | ||
name: Coveralls GitHub Action | ||
uses: coverallsapp/github-action@1.1.3 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
""" | ||
A set of tools for setting up OpenMM simulations of battery systems. | ||
""" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,175 @@ | ||
""" | ||
Concrete implementations of InputGenerators for the OpenMM IO. | ||
""" | ||
|
||
# base python | ||
from pathlib import Path | ||
from typing import Union, Optional, Dict, List, Tuple | ||
|
||
# scipy | ||
import numpy as np | ||
|
||
# openff | ||
|
||
# openmm | ||
from openmm.unit import kelvin, picoseconds | ||
from openmm import ( | ||
Context, | ||
LangevinMiddleIntegrator, | ||
) | ||
|
||
# pymatgen | ||
import pymatgen.core | ||
from pymatgen.io.core import InputSet, InputGenerator | ||
from pymatgen.io.openmm.inputs import ( | ||
TopologyInput, | ||
SystemInput, | ||
IntegratorInput, | ||
StateInput, | ||
) | ||
from pymatgen.io.openmm.sets import OpenMMSet | ||
from pymatgen.io.openmm.utils import ( | ||
get_box, | ||
get_coordinates, | ||
get_openmm_topology, | ||
parameterize_system, | ||
) | ||
|
||
__author__ = "Orion Cohen, Ryan Kingsbury" | ||
__version__ = "1.0" | ||
__maintainer__ = "Orion Cohen" | ||
__email__ = "orion@lbl.gov" | ||
__date__ = "Nov 2021" | ||
|
||
|
||
# noinspection PyMethodOverriding | ||
class OpenMMSolutionGen(InputGenerator): | ||
""" | ||
Generator for an OpenMM InputSet. | ||
|
||
This class is designed for generating simulations of mixed molecular systems. Starting with | ||
only SMILEs and counts, this class can generate a valid InputSet for a wide variety of | ||
molecular systems. Currently only the Sage force field is supported. | ||
|
||
This class is only compatible with the Langevin Middle Integrator. To use a different | ||
integrator, you can first generate the system with OpenMMSolutionGen and then add | ||
a different integrator to the OpenMMInputSet. | ||
""" | ||
|
||
def __init__( | ||
self, | ||
force_field: Union[str, Dict[str, str]] = "Sage", | ||
temperature: float = 298, | ||
step_size: float = 0.001, | ||
friction_coefficient: int = 1, | ||
partial_charge_method: str = "am1bcc", | ||
partial_charge_scaling: Optional[Dict[str, float]] = None, | ||
partial_charges: Optional[List[Tuple[Union[pymatgen.core.Molecule, str, Path], np.ndarray]]] = None, | ||
initial_geometries: Dict[str, Union[pymatgen.core.Molecule, str, Path]] = None, | ||
packmol_random_seed: int = -1, | ||
topology_file: Union[str, Path] = "topology.pdb", | ||
system_file: Union[str, Path] = "system.xml", | ||
integrator_file: Union[str, Path] = "integrator.xml", | ||
state_file: Union[str, Path] = "state.xml", | ||
): | ||
""" | ||
Instantiates an OpenMMSolutionGen. | ||
|
||
Args: | ||
force_field: force field for parameterization, currently supported Force Fields: 'Sage'. | ||
temperature: the temperature to be added to the integrator (Kelvin). | ||
step_size: the step size of the simulation (picoseconds). | ||
friction_coefficient: the friction coefficient which couples the system to | ||
the heat bath (inverse picoseconds). | ||
partial_charge_method: Any partial charge method supported by OpenFF Molecule.assign_partial_charges. | ||
"am1bcc" is recommended for small molecules, "mmff94" is recommended for large molecules. | ||
partial_charge_scaling: A dictionary of partial charge scaling for particular species. Keys | ||
are SMILEs and values are the scaling factor. | ||
partial_charges: A list of tuples, where the first element of each tuple is a molecular | ||
geometry and the second element is an array of charges. The geometry can be a | ||
pymatgen.Molecule or a path to an xyz file. The geometry and charges must have the | ||
same atom ordering. | ||
topology_file: Location to save the Topology PDB. | ||
system_file: Location to save the System xml. | ||
integrator_file: Location to save the Integrator xml. | ||
state_file: Location to save the State xml. | ||
""" | ||
self.force_field = force_field | ||
self.temperature = temperature | ||
self.step_size = step_size | ||
self.friction_coefficient = friction_coefficient | ||
self.partial_charge_method = partial_charge_method | ||
self.partial_charge_scaling = partial_charge_scaling if partial_charge_scaling else {} | ||
self.partial_charges = partial_charges if partial_charges else [] | ||
self.initial_geometries = initial_geometries if initial_geometries else {} | ||
self.packmol_random_seed = packmol_random_seed | ||
self.topology_file = topology_file | ||
self.system_file = system_file | ||
self.integrator_file = integrator_file | ||
self.state_file = state_file | ||
|
||
def get_input_set( # type: ignore | ||
self, | ||
smiles: Dict[str, int], | ||
density: Optional[float] = None, | ||
box: Optional[List[float]] = None, | ||
) -> InputSet: | ||
""" | ||
This executes all of the logic to create the input set. It generates coordinates, instantiates | ||
the OpenMM objects, serializes the OpenMM objects, and then returns an InputSet containing | ||
all information needed to generate a simulaiton. | ||
|
||
Please note that if the molecules are chiral, then the SMILEs must specify a | ||
particular stereochemistry. | ||
|
||
Args: | ||
smiles: keys are smiles and values are number of that molecule to pack | ||
density: the density of the system. density OR box must be given as an argument. | ||
box: list of [xlo, ylo, zlo, xhi, yhi, zhi]. density OR box must be given as an argument. | ||
|
||
Returns: | ||
an OpenMM.InputSet | ||
""" | ||
assert (density is None) ^ (box is None), "Density OR box must be included, but not both." | ||
smiles = {smile: count for smile, count in smiles.items() if count > 0} | ||
# create dynamic openmm objects with internal methods | ||
topology = get_openmm_topology(smiles) | ||
if box is None: | ||
box = get_box(smiles, density) # type: ignore | ||
coordinates = get_coordinates(smiles, box, self.packmol_random_seed, self.initial_geometries) | ||
smile_strings = list(smiles.keys()) | ||
system = parameterize_system( | ||
topology, | ||
smile_strings, | ||
box, | ||
self.force_field, | ||
self.partial_charge_method, | ||
self.partial_charge_scaling, | ||
self.partial_charges, | ||
) | ||
integrator = LangevinMiddleIntegrator( | ||
self.temperature * kelvin, | ||
self.friction_coefficient / picoseconds, | ||
self.step_size * picoseconds, | ||
) | ||
context = Context(system, integrator) | ||
context.setPositions(coordinates) | ||
state = context.getState(getPositions=True) | ||
# instantiate input files and feed to input_set | ||
topology_input = TopologyInput(topology) | ||
system_input = SystemInput(system) | ||
integrator_input = IntegratorInput(integrator) | ||
state_input = StateInput(state) | ||
input_set = OpenMMSet( | ||
inputs={ | ||
self.topology_file: topology_input, | ||
self.system_file: system_input, | ||
self.integrator_file: integrator_input, | ||
self.state_file: state_input, | ||
}, | ||
topology_file=self.topology_file, | ||
system_file=self.system_file, | ||
integrator_file=self.integrator_file, | ||
state_file=self.state_file, | ||
) | ||
return input_set |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Creating this
Context
caused me a truly nightmarish bug when trying to run on the Perlmutter GPUs. Somehow, instantiating aContext
creates some state in the program that persists even afterget_input_set
has run. Later on, when I callinput_set.get_simulation
, theSimulation
internally try's to create aContext
that conflicts with the previousContext
, causing an error and crashing the program. This does not occur locally when I useOpenCL
orCPU
as the platform, and it does not occur when I run remotely usingCPU
. Absolute nightmare.Unfortunately, we can't instantiate a
State
without creating aContext
, so these lines are integral to the current IO implementation.There is no issue if we create the input files in one python file and then load them in another, it is only an issue if we create and load them in the same file. So thankfully, this shouldn't be too much of an issue for HTMD.
I'm tempted to just ignore this issue and make sure the error is well documented. It's enough of an edge case that I don't think it warrants restructuring the IO. Curious to hear what you think though.
This was awful to debug, so I'm thankful to have gotten to the bottom of this error.