Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

One Sim, one Universe: remove multiple Universes functionality #47

Closed
dotsdl opened this issue Feb 22, 2016 · 15 comments
Closed

One Sim, one Universe: remove multiple Universes functionality #47

dotsdl opened this issue Feb 22, 2016 · 15 comments

Comments

@dotsdl
Copy link
Member

dotsdl commented Feb 22, 2016

Something that's become a clear problem from changes in upstream datreant.core is the state Sim instances carry with them, this being their "active" universe. The original idea for this was that for a given simulation one might have several different post-processed trajectories, and therefore perhaps different MDAnalysis.Universe definitions along with their own selections. It's not a bad idea, but it means that you will get something different with different instances of the same Sim when doing Sim.universe depending on what you've done previously.

This is an issue especially when working with Bundles of Sims, since doing set-style operations between different Bundles with overlapping sets of Sims (but not the same instances) means that one has no guarantee they get the Sim with the state they expect.

I propose to eliminate the "multiple Universes" functionality from Sim objects. This will reduce the state of a Sim to that stored in its state file (good!) and that stored in its loaded Universe instance (not great, but with new features coming in upstream MDAnalysis, could be mitigated). It will also simplify the Sim API greatly.

For those that use the "multiple Universes" functionality, (myself included), I think nesting Sim objects might be a good general solution. Something like:

main_Sim
    |-> Sim.<uuid>.json
    |
    |-> no_water
    |   |
    |   |->Sim.<uuid>.json
    |
    |-> fitted
        |
        |-> Sim.<uuid>.json

will work, and will be easy to use given work being done in upstream datreant.core.

Thoughts?

@dotsdl dotsdl added this to the 0.6.0 milestone Feb 22, 2016
@dotsdl dotsdl self-assigned this Feb 25, 2016
@dotsdl
Copy link
Member Author

dotsdl commented Feb 25, 2016

Moving forward with this one upon release of datreant.core.

@richardjgowers
Copy link
Contributor

Maybe it makes sense for there to be some sort of unsplittable object (ie how do I clearly know which the smallest matryoshka doll is). How is a nested Sim different from a Group of Sims?

@dotsdl
Copy link
Member Author

dotsdl commented Mar 6, 2016

Sorry @richardjgowers, just now plugging back into every loose thread.

I'm not sure I understand your question. Since Treants can have as much depth as the filesystem supports, this extends to Sims, and I was just noting a usage pattern that could work as an alternative to the "multiple Universes" functionality to be deprecated.

As for Groups, these are basically Treants with a persistent Bundle attached. They don't require "members" to be within their own tree, though that's allowed (and generally more robust when things move).

Does that help?

@richardjgowers
Copy link
Contributor

I was just thinking there might be value to creating an object (Sim) which is unambiguously singular. So when you hit this point, you know you've reached the end of the rabbit hole.

In terms of semantics, if I've run multiple simulations, I expect to have to use a container (Group?) of Sims.. not a Sim of Sims?

So wrt your diagram up top, I'd expect main_Sim to be a group not a sim.

I'm not sure if this breaks the entire model though, I don't really use MDS in complicated ways!

@dotsdl
Copy link
Member Author

dotsdl commented Mar 7, 2016

@richardjgowers functionally you could do either way. You could use a Group and just keep the related Sims together as members of it, or you could just nest the Sims. The point is that whatever you choose there is an alternative given that we are chopping off existing behavior of individual Sims.

@richardjgowers
Copy link
Contributor

something something something preferably one and only one way?

@dotsdl
Copy link
Member Author

dotsdl commented Mar 7, 2016

@richardjgowers fair enough. We considered axing Group entirely, but the conclusion there was it gives functionality one can't get with an in-memory aggregation (Bundle) or with nesting Treants. That discussion could be revisited, though, because there might be a better way entirely.

@richardjgowers
Copy link
Contributor

Sorry, I think I'm just playing devils avocado at this point. I just don't get how a Sim is multiple things, but maybe it makes sense with other data.

@orbeckst
Copy link
Contributor

I think support for axing multiple universes is universal.

Like @richardjgowers , I also find it conceptually cleaner to use a Group to perform (close to) the functionality of multiple universes than to nest Sims but this is a second-order question. If we're not 100% sure of the Matroshka-Sims then it might be sufficient for right now simply not to advertise the functionality, just in case we later introduce something that disallows a Sim to contain another Sim.

If this is the last thing between us an a datreant-compatible release of MDSynthesis then I suggest to agree quickly on the course of action.

@orbeckst
Copy link
Contributor

P.S.: I understand where @dotsdl is coming from with the hierarchical ordering in the nested Sims but the downside is that users have to think of yet another data structure in addition to Sim, Treant, Bundle, Group --- which is all well for developers but rather confusing for the casual user.

Or in MDS you shield people completely from the datreant stuff.... (which is probably more work and less pay-off for the user in the long term)

@dotsdl
Copy link
Member Author

dotsdl commented Mar 28, 2016

@orbeckst yeah, the delay has been mostly in implementation, which involves ripping things out of the Universes limb and putting them into the top-level namespace. I've started work on this, but haven't finished yet.

The main worry in keeping it is that the json schema is more complicated than it needs to be, so if we're going to break it, it needs to be now.

@orbeckst orbeckst removed the question label Mar 28, 2016
@orbeckst orbeckst changed the title One Sim, one Universe? One Sim, one Universe: remove multiple Universes functionality Mar 28, 2016
@orbeckst
Copy link
Contributor

Sure, break it now.
And I removed the question mark and the question from the issue ;-)

@dotsdl
Copy link
Member Author

dotsdl commented Mar 28, 2016

@orbeckst the hierarchical ordering is one alternative, but certainly not the only one. Basically, Treants can be nested, and this just works. One could probably use Groups similarly. Depends on individual needs/style; we don't prescribe anything here.

@dotsdl
Copy link
Member Author

dotsdl commented Mar 29, 2016

Aaaaand, they're gone: 7162a99

Need to update docs, then we'll make a release.

@dotsdl dotsdl closed this as completed Mar 29, 2016
@dotsdl
Copy link
Member Author

dotsdl commented Mar 29, 2016

Note: these changes break all Sims you may have been using up until this point. When the dust settles on the release I'll make a conversion script for getting usable Sims back out of existing ones. It should be easier to maintain changes in the future once we have releases to increment them on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants