shirtsgroup / physical_validation

Physical validation of molecular simulations
https://physical-validation.readthedocs.io
MIT License
55 stars 19 forks source link

Openmm parallel tempering example #103

Closed cwalker7 closed 3 years ago

cwalker7 commented 3 years ago

Description

First pass at the parallel tempering example (see #54 #50 ) using results from an Openmm simulation (run using the openmmtools framework (https://openmmtools.readthedocs.io/en/0.18.1/multistate.html). In the example, ensemble check is used on different pairs of temperature energies extracted from a .nc file which contains the energies of all replicas. The system used is a simple Lennard-Jones coarse-grained oligomer which undergoes a folding transition within the temperature range used. I also included a script for re-running the simulation with different parameters - the output nc files I uploaded are for a very short simulation (3ns).

I added a function in util called openmm_rep_exch which extracts the replica energies and replica state indices from the .nc file, and computes the constant temperature state energies, which are then used for physical validation. There are options to compute adjacent temperature pairs, all possible temperature pairs, or a single pair which best matches the maximum efficiency criteria.

Todos

Some important issues to sort out first.

  1. Something is going wrong with the imports after adding util.openmm_rep_exch. From the test we get this:

    ____ ERROR collecting physical_validation/tests/test_physical_validation.py ____
    ImportError while importing test module '/home/runner/work/physical_validation/physical_validation/physical_validation/tests/test_physical_validation.py'.
    Hint: make sure your test modules/packages have valid Python names.
    Traceback:
    /usr/share/miniconda/envs/test/lib/python3.6/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
    physical_validation/__init__.py:41: in <module>
    from . import data, ensemble, integrator, kinetic_energy, util
    physical_validation/data/__init__.py:31: in <module>
    from .ensemble_data import EnsembleData  # isort:skip
    physical_validation/data/ensemble_data.py:33: in <module>
    from ..util import error as pv_error
    physical_validation/util/__init__.py:31: in <module>
    from . import (
    physical_validation/util/openmm_rep_exch.py:37: in <module>
    from ..data.ensemble_data import EnsembleData
    E   ImportError: cannot import name 'EnsembleData'

    However it works fine if I make the __init__ files blank.

  2. Dependencies Openmmtools is used in util.openmm_rep_exch to parse the multiplexed .nc files, and the temperature list stored in the .nc files has simtk units. We probably don't want to have to include openmmtools as a dependency, and should decide if/how to remove this (I only added it to the testing environment for now to see if this worked).

    • [x] Fix import issues / decide where the openmm_rep_exch script belongs
    • [x] Remove dependency on simtk units and openmmtools if possible
    • [x] Check documentation and docstring formatting
    • [x] Add test for new functions? → New functions have been moved to examples, so no tests needed.

Questions

Status

ptmerz commented 3 years ago

As you suggested, we should avoid making the OpenMM tools a hard dependency. I think that the way to think about this is to define how we want OpenMM simulations to interact with physical validation in general (that is, not only for replica exchange simulations). In other words: How would I use physical validation if I have the results of one (kinetic tests) or two independent (ensemble tests) simple MD simulations in OpenMM. Does this require the tools?

If yes, we should probably hide the import statement within functions, and not make it a strict requirement.

If simple use does not require the tools, but replica exchange does, we should put any use of the tools into examples ("Here's how you can analyze replica exchange simulations in OpenMM"), and remove them from the util folder, which allows us to remove the dependency.

Refs #54 - we should probably first add a simpler example.

ptmerz commented 3 years ago

This is great work, thank you! Please let me know if my comment above misses a point, I have no experience using OpenMM (hence my thinking that getting a simpler example in first would help to define the actual requirements).

cwalker7 commented 3 years ago

If simple use does not require the tools, but replica exchange does, we should put any use of the tools into examples ("Here's how you can analyze replica exchange simulations in OpenMM"), and remove them from the util folder, which allows us to remove the dependency.

The way I've been using OpenMM, for a single simulation the energies are output as a csv text file, so we wouldn't need the extra tools to read it. So yeah, we could move the replica exchange script out of util and just have it in the examples. I agree the simple example would make things clearer.

@ocmadin do you still want to do the simple OpenMM example?

ocmadin commented 3 years ago

If simple use does not require the tools, but replica exchange does, we should put any use of the tools into examples ("Here's how you can analyze replica exchange simulations in OpenMM"), and remove them from the util folder, which allows us to remove the dependency.

The way I've been using OpenMM, for a single simulation the energies are output as a csv text file, so we wouldn't need the extra tools to read it. So yeah, we could move the replica exchange script out of util and just have it in the examples. I agree the simple example would make things clearer.

@ocmadin do you still want to do the simple OpenMM example?

Yeah, sorry I haven't gotten a chance to work on it yet. I'll try to put together an example by the end of this week, and if I don't have something (at least in progress, not necessarily a PR) by then, I'll let you have it.

codecov[bot] commented 3 years ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.88%. Comparing base (57c9a50) to head (36d4190).

Additional details and impacted files
ptmerz commented 3 years ago

@cwalker7, thank you again for contributing this. I have adapted your example to our new example format by putting it into a notebook. As discussed, I moved your script purely into the example, making what is happening more transparent to the reader and avoiding additional dependencies for physical_validation. The added example can be reviewed here: https://physical-validation--103.org.readthedocs.build/en/103/examples/openmm_replica_exchange.html

ptmerz commented 3 years ago

@cwalker7, I'll go ahead and merge this, so I can push another (hopefully final) RC. Please let me know if you feel we could improve something though, we can always add another PR! Thanks again, this is a great addition!

cwalker7 commented 3 years ago

@ptmerz Great, thank you for revisiting this - the documentation / example looks nice!