first release about mdsynthesis HOT 12 CLOSED

It's shipping today; working on docs and some final decisions for how limbs should work for a package like mdsynthesis to avoid namespace clashes with upstream limbs. Ran out of time yesterday before finishing this work up.

from mdsynthesis.

Docs are done as far as I can tell; please give a quick read and see if anything feels missing or off. I'm too involved to have fresh eyes on them: http://mdsynthesis.readthedocs.org/en/develop/

from mdsynthesis.

Not strictly related, but the links in the readme on the repo are a little borked. datreant_ has an underscore and there's a :ref: somewhere

from mdsynthesis.

@richardjgowers thanks for catching that. The README isn't the point of entry for users, but I did lift some of the rewritten intro text from the docs and put it there. Forgot to replace the sphinx-parsed bits with components that would work for raw RST.

from mdsynthesis.

docs:

1. I would make the first mentioning of Sim an actual link to the class definition
2. dependency: MDAnalysis >= 0.15? That's not even out yet...
3. Also make Sim a link in the Leveraging Sims section. Can you use intersphinx links to make MDAnalysis.Universe a link? (perhaps use ~MDAnalysis.core.AtomGroup.Universe?)
4. Treant and Sim are not always marked up.
5. Too much talking about Treant objects --- focus on the MDSynthesis user who comes to the package the first time and does not want to (or need to) dive into datreant. I think the docs will need some rewriting to get rid of the developer point of view but don't do this now... for the initial release it will be fine and it's not too bad. In fact, if you just say all the things about "Sims are Treants" after having explained what you do with Sims then it would be fine. (And don't explain Sims by contrasting with Treants... not helpful for people who don't know Treants.)
6. s.udef vs s.universe is very confusing: If I didn't know any better I'd do s.universe.trajectory = "path/to/xtc"... and it would even work with disastrous consequences. Perhaps making udef.trajectory a property is hiding to much (e.g. assigning reloads the universe?... that makes sense but seems to be a big side-effect; an explicit method call like load_trajectory() might be clearer)
7. Come to think, I guess I just dislike s.udef because neither the name nor the docs tell me what it is good for.
8. s.atomselections is really weird, syntactically and semantically: I assign a strong or a list and then I get back an AtomGroup?? I think you're abusing properties here but that might just be me. (At least you can assign an AtomGroup, so there's some balance.) I am not a fan of s.define(key) --- it seems decoupled from atomselections. But I don't have a good solution: making s.atomselections[key] a proper object allows one to do s.atomselections[key].definition = "selection string" (which is nice) but then s.atomselections[key].atoms, which seems clunky: just s.atomselections[key] would be nicer. Or make s.atoms[key] available for each s.atomselections[key].atoms...
9. mdsynthesis.limbs.AtomSelections: The name define() has the wrong semantics: You are not issuing a command to define but you are getting a definition back. It should be get_definition or perhaps just a dict-like lookup s.definitions[key] (and you're even raising a KeyError... hint, hint!)... and see above. When I encounter semantic shear in a UI (i.e. the function is different or the opposite from what the words mean in normal English) I cringe.

from mdsynthesis.

@dotsdl , it's very useful to have the UI laid out in the docs, so your docs are a success. But seeing it in this way, in context, in summary, made me realize that there are some UI choices that still feel rough or kludgey. Apologies if any or all of this had already been debated and settled previously.

from mdsynthesis.

docs:

• bottom: © Copyright 2014 ... shouldn't this be 2016?
• have a link to the GitHub repo on the first page in the text: you have datreant and MDAnalysis... but not MDS itself.

from mdsynthesis.

@orbeckst this is all extremely helpful. Thanks!

1. Fixed.
2. Oops...I clearly counted up one too far. I should know this. Set to 0.14
3. Yeah, we use intersphinx already; made a proper link to the module where Universe lives.
4. Should they be? I tend to find it distracting if they're always plaintexted or always links. If there's consensus to do this (e.g. instead of Treant, we always say Treant), then I'll do it.
5. This is a hard problem, and I don't have a good solution for it. Much of the non MD-specific functionality, which is to say 90% of what a Sim can do, is defined in datreant, and doing any kind of help(Sim.<methodname>) will do lots of talking about Treants, Trees, Views, etc. I think a downside to how datreant and MDSynthesis are designed is that you can't completely hide the inner library within the outer one without a lot of work that defeats the point (lots of duplication). All ears for how to sustainably go about this, though.
6. I agree its confusing, but a way of partitioning these things in the brain are that udef is the Sim's universe definition: the description of what it needs to generate its universe. The Sim's universe is the result of that definition, and it should always reflect that definition. Perhaps there are better ways of doing this, but part of the reason for this is that we want to limit how much Sim imlements into its own namespace to avoid clashes with upstream datreant. A Sim gives only three components in its namespace beyond what a Treant has: universe, udef, and atomselections. All three of these are unlikely to every clash with upstream limbs.
7. See above; if I make this way of viewing udef explicit in the docs, would that be a solution for this? Or should we completely rethink this, perhaps by monkey-patching the Universe such that Sim.universe.topology = 'path/to/top.psf' just works instead (or as well)?
8. I agree that this is asymmetric, and I don't like that generally. Alternatively one can always do s.atomselections.add('lid', 'resid 122:159') to achieve the same effect as s.atomselections['lid'] = 'resid 122:159'. Perhaps we could add an atoms limb as you suggest that gets back AtomGroups from stored atom selections, leaving atomselections to work only with the definitions? It'd be a bit different from previous limbs in that it interacts with the data stored by another, but it would work.
9. I agree; I've always hated the semantics of this name, but couldn't think of something that doesn't feel clunky (although they're traditional, I think get_* are a bit ugly). Depending on (8) above, this may be moot. Perhaps we should go that route, separating definition limb (atomselections) from object limb (atoms), in the same way as we do for the Universe (udef and universe)? That would bring some consistency on one dimension.

from mdsynthesis.

@dotsdl with 6 & 7, can we not just make udef hidden to the user and just make everything work through assigning a mda.Universe?

from mdsynthesis.

@richardjgowers I actually really like that idea, and I think we can go ahead with it and see how far we get. If it turns out we need a place for tweaking how the Universe behaves that can't be done from or with the Universe itself, then we can expose udef (remove the underscore, really).

This can go a long way toward including keyword argument persistence (see #25), since the solution arrived at for how the Universe stores these (MDAnalysis/mdanalysis#292) can be made to only have strings, bools, floats, and ints (some kwargs can be objects, so must be a mapping for those (I think we already have this for Readers)) so it is immediately extractable and storable by the Sim inside its JSON state file.

Sound good?

from mdsynthesis.

Alright, pushing to PyPI. Thanks all!

from mdsynthesis.

Hooray!

(And you even managed to close all issues in the tracker...)

from mdsynthesis.