search

choderalab / perses

Experiments with expanded ensembles to explore chemical space
http://perses.readthedocs.io
MIT License
178 stars 50 forks source link

Replace nonequilibrium integrators with NonequilibriumLangevinIntegrator #326

Open jchodera opened 7 years ago

jchodera commented 7 years ago

Now that we have NonequilibriumLangevinIntegrator in openmmtools, we should remove the redundant integrators from perses/annihilation/ncmc_switching.py.

jchodera commented 7 years ago

@pgrinaway : If we extend the integrator_type argument to NCMCEngine, should we keep the current terminology ([GHMC, VV, VVVR, BAOAB]) or standardize on the new integrator splitting language ([O { V R H R V } O, V R H R V, O V R H R V O, R V O H O V R])?

jchodera commented 7 years ago

I'd also like to add an ignore_shadow_work option, since we will presumably want to do so when g-BAOAB is in use.

I'm hoping we can reuse these parts of perses for at least some pilot experiments for the shadow work paper, but I think this refactoring would also be immediate use to you too. Did you want to tackle this or should I, @pgrinaway?

pgrinaway commented 7 years ago

If we extend the integrator_type argument to NCMCEngine, should we keep the current terminology ([GHMC, VV, VVVR, BAOAB]) or standardize on the new integrator splitting language ([O { V R H R V } O, V R H R V, O V R H R V O, R V O H O V R])?

Maybe for now we can use the current terminology, otherwise we'd have to add in extra checks (like make sure that people actually include a Hamiltonian update.

I'd also like to add an ignore_shadow_work option, since we will presumably want to do so when g-BAOAB is in use.

You mean disable its computation? That's probably a good thing to do to get efficiency. The integrator doesn't otherwise include shadow work in the protocol work variable, though.

I'm hoping we can reuse these parts of perses for at least some pilot experiments for the shadow work paper, but I think this refactoring would also be immediate use to you too. Did you want to tackle this or should I, @pgrinaway?

To be clear, by "this refactoring" you mean having perses use the integrators in openmmtools? I can tackle that.

jchodera commented 7 years ago

You mean disable its computation? That's probably a good thing to do to get efficiency. The integrator doesn't otherwise include shadow work in the protocol work variable, though.

Currently, NCMCEngine uses the total work (protocol + shadow). We want to be able to easily select whether NCMCEngine includes or omits shadow work.

To be clear, by "this refactoring" you mean having perses use the integrators in openmmtools? I can tackle that.

Yes!

pgrinaway commented 6 years ago

To clarify, this issue refers to refactoring the NCMCEngine, not the relative nonequilibrium stuff that we are doing now.