[WIP] Protocol : Nonequilibrium Cycling

[dotsdl]

Description

A nonequilibrium cycling protocol supporting execution with OpenMM, using the protocol structure defined in gufe.

Motivation and context

This is one of the first protocols to be developed using the gufe structure, and is intended to surface deficiencies in that structure. The end result will be a protocol that can be executed locally (with Protocol.execute()) as well as on cluster resources using a suitable Scheduler (e.g. an HPC scheduler like that being developed by OpenFE).

Immediate needs

• docstrings on all methods required or defined

How has this been tested?

• unit tests of protocol units
• unit tests of NonEquilibriumCyclingProtocol
• unit tests of NonEquilibriumCyclingResult

Change log

New features
^^^^^^^^^^^^
- `perses.protocols` submodule added; features `NonEquilibriumCyclingProtocol`

[codecov]

Codecov Report

Merging #1066 (71a94ba) into main (c96e655) will decrease coverage by 0.15%.
The diff coverage is 8.92%.

❗ Current head 71a94ba differs from pull request most recent head faac030. Consider uploading reports for the commit faac030 to get more accurate results

[zhang-ivy]

Suggestions for names other than Protocol: experiment, runner, workflow, pipeline

[dotsdl]

Ah, I think I'm still inclined to call this Protocol, since in the context of gufe this defines the procedure by which we execute the simulations necessary to calculate a \Delta G. A Protocol produces a ProtocolDAG from a set of input ChemicalSystems, an atom mapping (if necessary), and potentially any previous result that should be used as a starting point.

The ProtocolDAG is more akin to an individual experiment or workflow in this sense, while the Protocol it comes from is more akin to the instructions for performing it.

[zhang-ivy]

Thanks for the clarification! I still think it would be better to rename Protocol (i.e. to Procedure or InstructionSet) because if gufe is going to replace the perses API, "protocol" means something different in the context of free energy calculations, so its going to be confusing for perses users. However, it seems like I may be the only one that has a problem with the name, so I think its ok if we keep it as is and just make sure the class/name choice is well documented (which I'm sure you are planning to do anyway).

[ijpulidos]

I don't have a strong opinion here, there are valid points from all sides. I guess that as long as we make this clear from the docstrings and existing documentation, this should be fine. We may even want to put something like "Not to be confused with... " as a first line in the docstring.

I wonder if aliasing the gufe classes names is a sane option for perses codebase. I don't really know enough about design patterns around this.

[dotsdl]

...because if gufe is going to replace the perses API, "protocol" means something different in the context of free energy calculations, so its going to be confusing for perses users.

@zhang-ivy can you define here what "protocol" means in the context of free energy calculations? I don't know that there's a universally-accepted definition in the field, but if there is strong consensus I'd like to know what it is so we can act accordingly.

[zhang-ivy]

When I hear "protocol" in the context of alchemical free energy calculations, I think of the alchemical protocol, which tells us how we should transform the WT (or lambda = 0 endstate) into the mutant (or lambda = 1 endstate) residue. In practice, this can be thought of as a list of lambda values. The protocols I've been using are linear (e.g. if there are 4 alchemical states, the protocol would be [0, 0.25, 0.5, 1.0] and 0 would be mean that the WT residue's nonbonded interactions are fully on and the mutant residue's nonbonded interactions are fully off, 0.25 would mean that the WT residue's nonbonded interactions are turned 75% on and the mutant nonbonded interactions are turned 25% on, and so on), but they can be non-linear with a varying number of alchemical states.

The alchemical protocol used is critical to the calculation because it determines how well the alchemical states overlap, which in turn affects the bias and error of the calculation.

[dotsdl]

@ijpulidos can we sync up on this early this week? I'd like to be in a position to run this protocol locally, ideally by 11/08. Is there anything blocking this for you?

[mikemhenry]

Now that we are doing the refactor, we can merge this in once tests pass (feel free to remove tests that don't make sense anymore, but I do what the new stuff tested)

[ijpulidos]

Tests will fail with this one, I don't really know why they aren't being run. But I expect we would need to refactor some of the tests since the API for the RelativeFEPSetup object has changed.

Does it make sense that we try to merge this one as it is (after we resolve the merging conflicts) and work on fixing/refactoring the tests for it in a separate PR? Such that we can review these changes independently and it doesn't get too overwhelming (this is already a huge PR).

[ijpulidos]

Oh right, I think tests are not running because of the merge conflict, @mikemhenry already mentioned that in the past.

[ijpulidos]

We could just mark the failing tests to be skiped as old_api or similar. And fix the tests when we work on the refactor.

[ijpulidos]

The activity on this protocol has been migrated to https://github.com/choderalab/feflow. Closing.