Overshooting vs. Not overshooting: Consistency in DynamicsEngine.generate #95
Description
This is one of those issues which won’t cause any problems in practice, but should be handled if we’re going to claim that our code can handle any general path-sampling-like approach. I’m adding a tag "noturgent" to mark this and other questions like it. (Until we solve these, our code isn’t as general-purpose as we claim, but these bugs won’t break any likely approach.)
Depending on the nature of the continue conditions (which, in turn, typically depends on the nature of the ensemble), we can get one of two types of paths from DynamicsEngine.generate. To describe the problem, think in the context of having a continue condition based on Ensemble.can_append for some ensemble:
- If the path ensemble has
LengthEnsemblerequirements at the end (which is how we’re setting up our TIS ensemble, and all the ensembles I can think of from path sampling approaches can at least be written that way), then a path which could be accepted is exactly the path which is generated byDynamicsEngine.generate() - If the path ensemble does not have a
LengthEnsemblerequirement but does have a way to stop (e.g., if it ends with anInXEnsemble), then we have to overshoot beforecan_appendwill return false. In this case,generatereturns a trajectory which has one frame too many; we must trim that trajectory in order to accept it.
I see three possible solutions to this:
- Every path mover which uses
generatemust, if the trajectory returned bygenerateisn't acceptable, also check the trajectory with one fewer frame, to see if we overshot. This is easy to implement, but it is an ugly approach because it requires thePathMoverto modify the trajectory, which really should be the job of theDynamicsEngine. It also requires more work on the part of contributors who might addPathMovers. - Always overshoot. This is also easy to implement, but I don’t like it because it requires us to generate one extra frame for each trajectory.
- Split
can_appendinto two functions: one that really iscan_append, and one that isnot_overshot. Thedynamics_enginewould have to know whether it had to stop because it can’t append another trajectory, or whether the stop was because of an overshoot. If it overshot, then it would remove the extra frame and return the pre-overshoot trajectory. I think this is by far the best solution; the only problem is that it isn’t trivial to implement. If we agree that this is the way to go, I have a rough idea of how it would look, but still need to work out details.
Personally, I think this (like anything with the "noturgent" tag) is in the "not urgent but important" quadrant of the Eisenhower matrix, meaning we should come back to it before release but not before getting working TIS. The current implementation will work fine for TIS, TPS, and AMS (and probably for FFS, steered TPS, and anything else we’re putting into 1.0).
Activity
[-]Consistency in `DynamicsEngine.generate`[/-][+]Overshooting vs. Not overshooting: Consistency in `DynamicsEngine.generate`[/+]jhprinz commentedon Mar 4, 2016
I think this is related to #437.
dwhswenson commentedon Mar 5, 2016
Yes. I said that when I opened #437, didn't I?