Gromacs engine - #819

@hejung @jvreede Checking in with people who have been beta-testing this: Does it seem ready for production? If so, I'll remove the WIP status from it, and it will be ready for review.

Hi @dwhswenson . Where can I find an example notebook which uses Gromacs as an engine in OPS ?
I realized you had made one with alanine dipeptide but cant locate it online

@arjunwadhawan It's only in the branch for this PR -- since the Gromacs engine hasn't been merged into the master, it wouldn't make sense to have the example in master yet.

With a developer install of OPS, check out this branch (dwhswenson/gromacs_engine), and you'll see it in the examples. You can also find the notebooks (and related files) here: https://github.com/dwhswenson/openpathsampling/tree/gromacs_engine/examples/gromacs

Ah alright. Makes sense. We are going to use it and will get back if any issue
Thanks

Recent commits address stuff discussed in #855.

The most recent commits are not backwards compatible with files generated by earlier versions of the OPS Gromacs engine. Here's the explanation and the workaround:

What changed and why?

Now OPS will store the contents files used to set up the simulation (e.g., .gro, .mdp, .top) inside the engine when saving the engine to our .nc storage.

OPS simulation objects should be immutable (at least, mostly). When you reuse an engine, you should always get the same behavior.

However, until these commits you could reload an Gromacs engine from storage, and change the behavior of the engine by changing the files. For example, you could change the integrator by editing the mdp file.

This is a problem because we want to ensure that you have a record of provenance in your data: you should to be able to tell exactly what integrator was used to create a specific snapshot.

In the new implementation, OPS will check for the existence of a file with the correct name. If it doesn't exist, OPS will create the file (writing the stored contents to disk). If it does exist, OPS will check whether the sha1 hash matches the hash of the stored data. If so, it uses the existing file. If not, it raises an error.

We raise an error on mismatch because we're not sure whether wanted to use the engine as is (in which case, maybe you don't want to overwrite the file?) or if you thought that you could change the behavior by changing the file (which breaks immutability and provenance checking).

Why will this raise an error with existing storages?

Before these commits, the file contents weren't stored. When OPS tries to reload the contents of these files from our nc storage, the engine will fail to load. For some reason, it appears to fail silently, too -- so you'll get None for the engine. (This is behavior that will be different in the new storage.)

How can I work around the error?

Write a custom from_dict function and monkey patch it to the engine class (before reloading!):

from openpathsampling.engines import gromacs as ops_gmx
def custom_from_dict(cls, dct):
    return super(ops_gmx.Engine, cls).from_dict(dct)

ops_gmx.Engine.from_dict = classmethod(custom_from_dict)

Note that after monkey-patching, you will (within a given Python session) only be able to load old-format engines, but will store in the new format.

Tagging people who might need to know this: @hejung @arjunwadhawan @IdsTeepe

I'm calling this one ready for review (FINALLY!!) The last changes were to move the ExternalMDSnapshot out of Gromacs-specific code (because it can be used by other engines, too) and to make the ExternalEngine abstract, putting a subclass with the reference/test implementation in the test suite.

This is a huge PR, and includes #144, which will not be merged separately. I'll leave this up for a week for review, merging no earlier than Saturday 7 December. If anyone is actually going to review it and wants longer than that, let me know. If there are no comments expressing an interest in reviewing, I'll assume everything is okay, and will merge.

I plan to release 1.1 shortly after merging this.

Thanks to @hejung and @IdsTeepe for doing some real-world testing of this PR. Just to make sure everyone who might care is getting the message: @jhprinz @arjunwadhawan.