Add external work alchemical Langevin splitting integrator

[sgill2]

This PR aims to create an integrator that inherits from the openmmtools NonequilibriumLangevinIntegrator so that it also allows the non-alchemical protocol work to be measured.

• get integrator working
• write test to make sure it's handling the external work correctly

[davidlmobley]

Tagging @pgrinaway

[sgill2]

For a test I'll probably do something like the following:
1.take an atom from an alanine dipeptide and distort it in a fixed way at some point in the middle of the nsteps_neq, while keeping lambdas constant.
2.Assert that the protocol work != 0 (or maybe protocol work ~= some constant)

[davidlmobley]

@sgill2 - you changed the title, is this ready for review?

[davidlmobley]

Also (not having looked at the code changes yet), as per the description of your PR, this adds an integrator, but does it switch the main code to use this integrator, or is the integrator "vestigial" (not used)? It would be good to document more completely here in this PR (and, if it's functional, in the relevant documentation). If it's just "vestigial" you'll want to create a separate issue(s) explaining what is going to be done with this from here.

[sgill2]

Also (not having looked at the code changes yet), as per the description of your PR, this adds an integrator, but does it switch the main code to use this integrator, or is the integrator "vestigial" (not used)? It would be good to document more completely here in this PR (and, if it's functional, in the relevant documentation). If it's just "vestigial" you'll want to create a separate issue(s) explaining what is going to be done with this from here.

I suppose that's worth discussing now.
Going from what John says about the splitting integrators (http://rspa.royalsocietypublishing.org/content/472/2189/20160138), it seems like having the BAOAB integration scheme as the default would be the best way to go for now. I'd say replace the VV integrator default with this integrator in the PR.

[sgill2]

But yeah, this should be about ready to review @davidlmobley. If @pgrinaway has a moment I'd also especially appreciate his input too.

[davidlmobley]

@sgill2 :

I suppose that's worth discussing now. Going from what John says about the splitting integrators (http://rspa.royalsocietypublishing.org/content/472/2189/20160138), it seems like having the BAOAB integration scheme as the default would be the best way to go for now. I'd say replace the VV integrator default with this integrator in the PR.

Have you or Nathan done any tests on this yet to see what it does to acceptance, etc.?

(Though I think this also relates to the other issue we're currently discussing, in that if you go to an integrator in openmmtools you reduce code duplication.)

[davidlmobley]

Per discussion, this is going to be vestigial for now (see #50 and MMC) so the doc strings will need updating to indicate that.

[pgrinaway]

It looks good to me!

[nathanmlim]

Overall looks good to me.

I was going to suggest that the tests should be written in the unittest framework as the Orion cubes and the tests I had written for the refactored code used the unittest framework. But after some reading, I think the framework you had written here and in previous tests is actually much easier with using the pytest framework.

I'll raise an issue to change the tests I had written up (since they're bad anyways).

[davidlmobley]

OK, looks/sounds good, guys. Though we either need to update the doc strings to indicate this is vestigial (ideally with a link to this discussion) before merging this PR, or we need to create a separate issue to do so.

[nathanmlim]

I'm of the side we do it as a separate PR.

[davidlmobley]

OK, sounds fine.