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

Residue.mass and Residue.charge use deprecated numpy indexing #2990

Closed
jbarnoud opened this issue Oct 16, 2020 · 0 comments · Fixed by #2991
Closed

Residue.mass and Residue.charge use deprecated numpy indexing #2990

jbarnoud opened this issue Oct 16, 2020 · 0 comments · Fixed by #2991

Comments

@jbarnoud
Copy link
Contributor

Running the tests throws a warning about MDAnalysis using a behaviour of numpy that is subject to change:

MDAnalysisTests/core/test_accumulate.py::TestTotals::test_total_charge_duplicates[residues]
  /home/jon/dev/mdanalysis/package/MDAnalysis/core/topologyattrs.py:1540: FutureWarning: Using a non-tuple sequence for multidimensional indexing is deprecated; use `arr[tuple(seq)]` instead of `arr[seq]`. In the future this will be interpreted as an array index, `arr[np.array(seq)]`, which will result either in an error or a different result.
    charges = self.values[resatoms].sum()

MDAnalysisTests/core/test_accumulate.py::TestTotals::test_total_mass_duplicates[residues]
  /home/jon/dev/mdanalysis/package/MDAnalysis/core/topologyattrs.py:1119: FutureWarning: Using a non-tuple sequence for multidimensional indexing is deprecated; use `arr[tuple(seq)]` instead of `arr[seq]`. In the future this will be interpreted as an array index, `arr[np.array(seq)]`, which will result either in an error or a different result.
    masses = self.values[resatoms].sum()

The warnings are thrown for core/test_accumulate.py::TestTotals::test_total_charge (and its mass equivalent):

ref = group.total_charge() + group[0].charge

The problematic call is a call to Residue.charge or Residue.mass (note the singular group name). This calls topologyattrs.Charges.get_residues which makes a problematic array indexing on line

charges = self.values[resatoms].sum()
or topologyattrs.Masses.get_residues on line
masses = self.values[resatoms].sum()

Here, an array is indexed with a list of arrays, hence the warning.

The get_segments counterpart are fixed and convert the list of arrays into a tuple of arrays, not triggering the warning.

Code to reproduce the behavior

import MDAnalysis as mda
from MDAnalysis.tests.datafiles import XTC, TRR

u = mda.Universe(TRR, XTC)
u.residues[0].mass  # or u.residues[0].charge

or

pytest core/test_accumulate.py::TestTotals

Current version of MDAnalysis

  • Which version are you using? (run python -c "import MDAnalysis as mda; print(mda.__version__)") 2.0.0-dev0 64a7c05
  • Which version of Python (python -V)? Python 3.8.5
  • Which operating system? Ubuntu 20.04 on WSL
jbarnoud added a commit that referenced this issue Oct 16, 2020
Fixes #2990

Avoid indexing a numpy array with a list of arrays, use a tuple of array
instead. Multidimensional indexing of arrays with a non-tuple argument
is deprecated.
orbeckst pushed a commit that referenced this issue Oct 16, 2020
* Fixes #2990
* Update indexing in Charges and Masses get_residues: 
   Avoid indexing a numpy array with a list of arrays, use a tuple of array
   instead. Multidimensional indexing of arrays with a non-tuple argument
   is deprecated.
* Turn warnings into errors to avoid regression over #2990
* Update CHANGELOG
PicoCentauri pushed a commit to PicoCentauri/mdanalysis that referenced this issue Mar 30, 2021
* Fixes MDAnalysis#2990
* Update indexing in Charges and Masses get_residues: 
   Avoid indexing a numpy array with a list of arrays, use a tuple of array
   instead. Multidimensional indexing of arrays with a non-tuple argument
   is deprecated.
* Turn warnings into errors to avoid regression over MDAnalysis#2990
* Update CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant