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

HydrogenBondAnalysis: KeyError for heavy atoms when selection is updated #1687

Open
danijoo opened this Issue Oct 26, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@danijoo

danijoo commented Oct 26, 2017

Expected behaviour

When I want to get hydrogen bonding frequencies (h.count_by_type()) in combination with the update_selections flag, it should built the proper frequencies table.

Actual behaviour

Under some circumstances, if update_selection is set to true, a given index might not be present in self._s2_donors when the heavy atom lookup table is built. This leads to a crash when the heavy atoms are generated because the key is not present in the lookup table. This happens mostly when the selection that is set to update includes solvent or other rapid exchanging residues.

Code to reproduce the behaviour

import MDAnalysis as mda
u = mda.Universe(top, trj)

# this works
h = mda.analysis.hbonds.HydrogenBondAnalysis(u, "resid 1-5", "resname SOL", update_selection1=False, update_selection2=False)
h.run()
h.generate_table()
frequencies = h.count_by_type()

# this fails
h = mda.analysis.hbonds.HydrogenBondAnalysis(u, "resid 1-5", "resname SOL", update_selection1=False, update_selection2=True)
h.run()
h.generate_table()
frequencies = h.count_by_type()
 
Traceback (most recent call last):

  File "<ipython-input-7-438a429831da>", line 16, in <module>
    frequencies = h.count_by_type()

  File "/home/bauer/.local/lib/python3.6/site-packages/MDAnalysis/analysis/hbonds/hbond_analysis.py", line 1149, in count_by_type
    r.donor_heavy_atom[:] = [h2donor[idx] for idx in r.donor_index]

  File "/home/bauer/.local/lib/python3.6/site-packages/MDAnalysis/analysis/hbonds/hbond_analysis.py", line 1149, in <listcomp>
    r.donor_heavy_atom[:] = [h2donor[idx] for idx in r.donor_index]

KeyError: 29971

Currently version of MDAnalysis:

import sys
import MDAnalysis as md
md.__version__
# '0.16.0'
sys.version
# '3.6.2 (default, Jul 20 2017, 03:52:27) \n[GCC 7.1.1 20170630]'
 
@orbeckst

This comment has been minimized.

Show comment
Hide comment
@orbeckst

orbeckst Oct 27, 2017

Member

@danijoo are you able to reproduce the error with the latest release 0.16.2, too?

(It is generally safe to upgrade within the minor release number and there were various bug fixes 0.16.0 --> 0.16.2 and there was a fix for HBanalysis although it is likely not fixing your problem, but still, we need to work from the latest release.)

Are you able to reproduce the error with one of the existing trajectories, e.g.,

from MDAnalysis.tests.datafiles import TPR, XTC

If yes, then it is a lot easier for a developer to have a look at it. I would consider providing a repoducible failure the biggest contribution a user can make to fixing a bug. My experience is that with a reproducible failure, developers are much more likely to "just have a quick look" and then actually get to fixing it. On the other hand, finding a test/failure first is probably the number one reason why issues don't get addressed promptly.

Member

orbeckst commented Oct 27, 2017

@danijoo are you able to reproduce the error with the latest release 0.16.2, too?

(It is generally safe to upgrade within the minor release number and there were various bug fixes 0.16.0 --> 0.16.2 and there was a fix for HBanalysis although it is likely not fixing your problem, but still, we need to work from the latest release.)

Are you able to reproduce the error with one of the existing trajectories, e.g.,

from MDAnalysis.tests.datafiles import TPR, XTC

If yes, then it is a lot easier for a developer to have a look at it. I would consider providing a repoducible failure the biggest contribution a user can make to fixing a bug. My experience is that with a reproducible failure, developers are much more likely to "just have a quick look" and then actually get to fixing it. On the other hand, finding a test/failure first is probably the number one reason why issues don't get addressed promptly.

@orbeckst

This comment has been minimized.

Show comment
Hide comment
@orbeckst

orbeckst Nov 7, 2017

Member

ping @danijoo – it would really help if you could provide more information and address the questions. Thanks.

Member

orbeckst commented Nov 7, 2017

ping @danijoo – it would really help if you could provide more information and address the questions. Thanks.

@orbeckst orbeckst added the defect label Dec 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment