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

Using Sim selections #31

Closed
richardjgowers opened this issue Jun 22, 2015 · 3 comments
Closed

Using Sim selections #31

richardjgowers opened this issue Jun 22, 2015 · 3 comments
Milestone

Comments

@richardjgowers
Copy link
Contributor

I was playing with the selections tool on Sims, and I bumped into a couple annoyances

ag = u.atoms[:10]

S.selections.add(ag)

# TypeError: object name is not a string: <AtomGroup with 10 atoms>

I guess this ties in with #25 in accepting premade MDA objects. This should be possible as AtomGroups are unambiguous based on a hash of their Universe (to uniquely identify this among S.universes) and then just ag.indices()

S.selections += 'something'

# TypeError: unsupported operand type(s) for +=: 'Selections' and 'str'

Using .add() feels weird to me, intuitively I want to += stuff in. Thoughts?

@dotsdl
Copy link
Member

dotsdl commented Jun 22, 2015

You have to give selections a name in order to generate them later; the usage should look like:

ag = 'bynum 0:10'

S.selections.add('my favorite atoms', ag)

Selections are stored as (lists of) selection strings, and they are applied on call to give back an AtomGroup from the active universe. I think it should be possible to feed AtomGroups in directly instead of selection strings, storing a list of atom indices instead of strings for the selection definition.

BTW, you can avoid the (admittedly awkward) add syntax by using setitem syntax:

S.selections['my favorite atoms'] = 'bynum 1:10'

and get the corresponding AtomGroup from the selection definition with:

ag = S.selections['my favorite atoms']

I don't know if there's a clean way to handle += as an operation, especially if some selections are defined as lists of strings while others are defined by lists of atoms. I think maintaining a predictable atom order would get very tricky real fast.

@richardjgowers
Copy link
Contributor Author

Yeah I don't mind hacking in adding AtomGroups directly myself, I hate working in terms of selection strings myself.

For the second point, there's the python method __iadd__ (and friends), which lets you handle augmented assignment in special ways.

https://docs.python.org/2/library/operator.html#operator.iadd

I forgot they're all key: value pairs, so possible things you could do

S.selections += {'something': 'sel 1', 'somethign else': 'sel 2'}

S.selections += 'key', 'value'

Food for thought at least

@dotsdl dotsdl added this to the 0.6.0 milestone Jun 22, 2015
@dotsdl
Copy link
Member

dotsdl commented Jun 22, 2015

I don't mind adding in these kind of enhancements so long as they can be unambiguously implemented at both the interface and persistence levels. I would suggest the following:

  1. add a new table definition to persistence.SimFile for _SelectionAtoms, which stores integers in a single column representing the atom indices of the selection
  2. add logic to persistence.SimFile.add_selection to create a _SelectionAtoms table for integers given as selection, as well as the usual _Selection table for selection strings given as selection (current behavior)
  3. add logic to aggregators.Selections add and __setitem__ (or better, change __setitem__ to call add, and change only add) to handle AtomGroups. That is, to pull out their atom indexes and feed them to self._backend.add_selection

I prefer to keep MDAnalysis calls out of persistence as much as possible to limit the complexity of the File classes; this separation preserves that.

richardjgowers added a commit to richardjgowers/MDSynthesis that referenced this issue Jun 25, 2015
Closes Issue datreant#31

Changed core/aggregators/Selection getitem to route through add method

Added _SelectionAtoms type to core/persistence/SimFile for storing
indices

core/persistence/SimFile add_selection can now add either selection
strings or indices arrays (new).
richardjgowers added a commit to richardjgowers/MDSynthesis that referenced this issue Jun 25, 2015
Closes Issue datreant#31

Changed core/aggregators/Selection getitem to route through add method

Added _SelectionAtoms type to core/persistence/SimFile for storing
indices

core/persistence/SimFile add_selection can now add either selection
strings or indices arrays (new).
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

2 participants