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
LengthEnsemble requirements 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 by DynamicsEngine.generate()
- If the path ensemble does not have a
LengthEnsemble requirement but does have a way to stop (e.g., if it ends with an InXEnsemble), then we have to overshoot before can_append will return false. In this case, generate returns 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
generate must, if the trajectory returned by generate isn'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 the PathMover to modify the trajectory, which really should be the job of the DynamicsEngine. It also requires more work on the part of contributors who might add PathMovers.
- 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_append into two functions: one that really is can_append, and one that is not_overshot. The dynamics_engine would 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).
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 onEnsemble.can_appendfor some ensemble: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()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:
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.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).