Repository navigation
Toy Dynamics should have separate System object #73
Description
Activity
I am not 100% sure what you mean. You are not saying that toy_dynamics should avoid using snapshots? So I assume that current_system means a particular simulation out of many?
If so, I would rather use several simulation objects.
Sorry for being unclear; this is just meant to address the "best practices" conversation from #65, especially so that ToyDynamics actually implements the four objects enumerated in #65 (comment). I think we agreed that this 4-object model was a best practice, though we only wanted to force the users to implement a minimal interface. However, we should use that 4-object model in our own things (unless there's a good reason not to), so that we set a good example for people learning from our code. In particular, ToyDynamics really must use the 4-object model, because there's not a single good reason for it not to.
Currently, ToyDynamics uses a 3-object model, which objects 3 and 4 from the 4-object model (simulation state and overall simulator) are lumped together. This works, but is not the most pedagogically useful approach. (The entire reason I did it that way was laziness, which does not count as a good reason.) This issue is just intended to remind me to bring ToyDynamics into accord with the 4-object model.
Internally, ToySimulation does not use
Snapshots because it doesn't have any concept of atoms. It sees the entire position array as a single vector moving on a pre-defined global potential energy surface (as opposed to a PES defined by interaction potentials).In OpenMM terms,
ToySimulation().current_system(maybe better to call itcurrent_system_state?) is intended to be likeopenmm.app.simulation.context.If simultaneously running multiple trajectories (with everything else the same), I think it makes sense to have multiple simulation objects. I think we're discussing this idea in #65, so I'll put my comments on it there.
As mentioned in #65 (comment), when this gets implemented, I should also make sure that all the information which needs to be stored about the simulation goes into its
__dict__.What is the status on this. Do we still need to make changes here?
Milestoned future. It should be done, but not in 1.0.
I think we can close this as it is replaced by using features and the new Snapshot factory.
I think we can close this as it is replaced by using features and the new Snapshot factory.
No, that misses the intent. See #73 (comment).
Thanks for reminding me.
Per discussion in #65 (comment) ff., Toy Dynamics should have a separate object (perhaps called
ToySystemorToySystemState), such that forsimulation = ToySimulation(), instead ofwe have
This makes Toy Dynamics more pedagogically useful