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

Set operations for Groups #726

Closed
khuston opened this issue Feb 16, 2016 · 13 comments
Closed

Set operations for Groups #726

khuston opened this issue Feb 16, 2016 · 13 comments

Comments

@khuston
Copy link
Contributor

khuston commented Feb 16, 2016

I apologize if this is already a feature or if has been discussed before, but I've often thought AtomGroup subtraction would be a convenient feature, in the same way __add__ is already overridden for AtomGroup. It would be defined in the same way set subtraction is defined for Python:

For AtomGroup instances:

return AtomGroup(list(set(self._atoms) - set(other._atoms)))

For Atom instances:

return AtomGroup(list(set(self._atoms) - set([other])))

It's a simple addition, but it seems to work. If it's alright, I can submit a pull request with the feature and tests.
Example usage:

In [2]: u.select_atoms('resname 58')
Out[2]: <AtomGroup with 300 atoms>

In [3]: u.select_atoms('resname 58 and prop z < 220')
Out[3]: <AtomGroup with 201 atoms>

In [4]: u.select_atoms('resname 58') - u.select_atoms('resname 58 and prop z < 220')
Out[4]: <AtomGroup with 99 atoms>
@richardjgowers
Copy link
Member

Hey Kyle,

Yeah I think this would be great. I think @jbarnoud ? recently proposed this somewhere else, but I can't find it for the life of me.

We're changing how AtomGroups work quite soon, but I think most of the hard work here is coming up with solid tests (which won't change) rather than the actual AtomGroup methods (which will change). So yeah, put together something

@jbarnoud
Copy link
Contributor

I have a work in progress based on the new topology scheme. I implemented all the set operators and I am working at testing them.

The current status is visible at https://github.com/jbarnoud/mdanalysis/commits/issue-363-set-operators.

@dotsdl
Copy link
Member

dotsdl commented Feb 16, 2016

Since AtomGroup is not technically a set, how are repeats handled? Are repeats just dropped and the result just sorted?

@mnmelo
Copy link
Member

mnmelo commented Feb 16, 2016

As a workaround while we decide and develop you can use:

# to emulate difference = grp1 - grp2
difference = grp1.select_atoms("not group other", other=grp2)

@jbarnoud
Copy link
Contributor

@dotsdl Yes. When using set operations, the repeats are dropped in the result, and the index are sorted.

@khuston
Copy link
Contributor Author

khuston commented Feb 17, 2016

Thank you for that @mnmelo - I didn't know about the other keyword.

So since __eq__ for Atoms is based on the index, atoms with the same index will be dropped. I forgot about the case where a user adds AtomGroups together to get an AtomGroup with duplicate atoms.

@mnmelo
Copy link
Member

mnmelo commented Feb 17, 2016

As a clarification on the workaround: group is the keyword. other is an arbitrary name that must match a named argument to select_atoms (trhough which you pass an existing AtomGroup). You might as well write:

difference = grp1.select_atoms("not group khuston", khuston=grp2)

@richardjgowers
Copy link
Member

One option here is to not use the - operator, but only allow it via a difference method. This way it's more explicit that a set difference operation has been done, rather than another interpretation of subtraction.

@jbarnoud
Copy link
Contributor

I really like the idea of having all set operators available, and not only the corresponding functions. But I have to agree that, in the particular case of - may be confusing because of the asymmetry with + that do keep the duplicates.

@jbarnoud jbarnoud removed their assignment Feb 18, 2016
@orbeckst
Copy link
Member

My expectation for "-" would be that it

  • keeps the original order
  • removes any atom that is found in the subtracted group

i.e.,

def subtract(a, b):
    return [idx for idx in a.indices if idx in b.indices]

Admittedly, these are not set operations but AtomGroups are not sets either so I feel it's ok to be idiosyncratic. I would reserve explicit methods such as g.difference() or g.union() for actual set operations.

@richardjgowers
Copy link
Member

@orbeckst So for example?

ag1 = [1, 2, 2, 5, 4, 3]

ag2 = [2, 4]

ag1 - ag2 = [1, 5, 3]

That makes sense to me (and is the opposite to +), and then having the set operations explicit via methods makes that clear too.

@orbeckst
Copy link
Member

Yes, that's what makes sense to me.

Oliver Beckstein
email: [email protected]

Am Feb 23, 2016 um 3:24 schrieb Richard Gowers [email protected]:

@orbeckst So for example?

ag1 = [1, 2, 2, 5, 4, 3]

ag2 = [2, 4]

ag1 - ag2 = [1, 5, 3]
That makes sense to me (and is the opposite to +), and then having the set operations explicit via methods makes that clear too.


Reply to this email directly or view it on GitHub.

@orbeckst orbeckst added this to the Topology refactor milestone Mar 18, 2016
@richardjgowers richardjgowers changed the title set subtraction for AtomGroups Set operations for Groups Mar 8, 2017
@richardjgowers
Copy link
Member

@jbarnoud did some cool code here:

jbarnoud@63c1f2b

Should we add these directly as Group methods? Or have a setoperations submodule in MDAnalysis?

@jbarnoud jbarnoud self-assigned this Mar 8, 2017
jbarnoud added a commit that referenced this issue Mar 9, 2017
jbarnoud added a commit that referenced this issue Mar 13, 2017
See #726

This commit introduce several method to the GroupBase class, including
all the set operators, and a subtract method. Some operator symbols are overridden.
jbarnoud added a commit that referenced this issue Mar 15, 2017
See #726

This commit introduce several method to the GroupBase class, including
all the set operators, and a subtract method. Some operator symbols are
overridden.
orbeckst added a commit that referenced this issue Mar 15, 2017
Introduce operators to groups

Fixes #726
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

6 participants