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

Some methods that accept the compound keyword fail or don't return the expected set of values #3002

Closed
mnmelo opened this issue Oct 20, 2020 · 0 comments · Fixed by #3905
Closed

Comments

@mnmelo
Copy link
Member

mnmelo commented Oct 20, 2020

For at least moment_of_inertia and asphericity the use of compounds seems to be broken:

Expected behavior

from MDAnalysis.tests.datafiles import TRZ_psf, TRZ
u = mda.Universe(TRZ_psf, TRZ) # universe with 24 residues

u.atoms.asphericity(compound='residues')
# expected a 1D array with 24 values of asphericity, one per residue
# equivalent to
np.array([res.atoms.asphericity() for res in u.residues])

u.atoms.moment_of_inertia(compound='residues')
# expected a 3D array with 24 MoI 3x3 tensors, one per residue, with shape (24,3,3)
# equivalent to
np.array([res.atoms.moment_of_inertia() for res in u.residues])

Actual behavior

u.atoms.asphericity(compound='residues')
~/src/MDAnalysis/mdanalysis/package/MDAnalysis/lib/util.py in wrapper(group, *args, **kwargs)
   1841         # has already been thrown:
   1842         if group.isunique or warn_if_not_unique.warned:
-> 1843             return groupmethod(group, *args, **kwargs)
   1844         # Otherwise, throw a DuplicateWarning and execute the method.
   1845         method_name = ".".join(

~/src/MDAnalysis/mdanalysis/package/MDAnalysis/core/groups.py in wrapped(group, *args, **kwargs)
    386                 raise ValueError(
    387                     "both 'pbc' and 'unwrap' can not be set to true")
--> 388         return function(group, *args, **kwargs)
    389     return wrapped
    390

~/src/MDAnalysis/mdanalysis/package/MDAnalysis/core/topologyattrs.py in asphericity(group, pbc, unwrap, compound)
   1418             pbc=pbc, unwrap=unwrap, compound=compound)
   1419         if compound != 'group':
-> 1420             com = (com * group.masses[:, None]
   1421                    ).sum(axis=0) / group.masses.sum()
   1422

ValueError: operands could not be broadcast together with shapes (24,3) (8184,1)
u.atoms.moment_of_inertia(compound='residues')
~/src/MDAnalysis/mdanalysis/package/MDAnalysis/lib/util.py in wrapper(group, *args, **kwargs)
   1841         # has already been thrown:
   1842         if group.isunique or warn_if_not_unique.warned:
-> 1843             return groupmethod(group, *args, **kwargs)
   1844         # Otherwise, throw a DuplicateWarning and execute the method.
   1845         method_name = ".".join(

~/src/MDAnalysis/mdanalysis/package/MDAnalysis/core/groups.py in wrapped(group, *args, **kwargs)
    386                 raise ValueError(
    387                     "both 'pbc' and 'unwrap' can not be set to true")
--> 388         return function(group, *args, **kwargs)
    389     return wrapped
    390

~/src/MDAnalysis/mdanalysis/package/MDAnalysis/core/topologyattrs.py in moment_of_inertia(group, pbc, **kwargs)
   1260             pbc=pbc, unwrap=unwrap, compound=compound)
   1261         if compound != 'group':
-> 1262             com = (com * group.masses[:, None]
   1263                    ).sum(axis=0) / group.masses.sum()
   1264

ValueError: operands could not be broadcast together with shapes (24,3) (8184,1)

Calling the method on a ResidueGroup seems to default to using compound='group' and yield a single value. More puzzingly, calling it on a ResidueGroup and explicitly passing compound='residues' works (no ValueError) but yields the same single value (with some precision difference betraying a different logic route):

u.residues.asphericity()
>> 0.5655677527570107

u.residues.asphericity(compound='residues')
>> 0.5655677527570102
u.residues.moment_of_inertia()
>> array([[1332923.85838838,  480989.6752955 , -227811.7564208 ],
          [ 480989.6752955 ,  612813.7537505 ,  231931.75813526],
          [-227811.7564208 ,  231931.75813526, 1509650.41986839]])

u.residues.moment_of_inertia(compound='residues')
>> array([[1332923.85838838,  480989.6752955 , -227811.7564208 ],
          [ 480989.6752955 ,  612813.7537505 ,  231931.75813526],
          [-227811.7564208 ,  231931.75813526, 1509650.41986839]])```

A consolidated compound-splitting strategy (#3000) might help fix these issues and uniformize the API.

Current version of MDAnalysis

  • Which version are you using? 2.0.0-dev0 (at commit beb5232)
  • Which version of Python? 3.6.9
  • Which operating system? Ubuntu 18.04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants