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

AtomGroup.unwrap confused about bound sub-groups #3352

Closed
mnmelo opened this issue Jun 10, 2021 · 3 comments
Closed

AtomGroup.unwrap confused about bound sub-groups #3352

mnmelo opened this issue Jun 10, 2021 · 3 comments

Comments

@mnmelo
Copy link
Member

mnmelo commented Jun 10, 2021

Expected behavior

ag.unwrap should make the group's fragments whole. It does in version 1.1.1.

Actual behavior

In 2.0.0-dev0 an exception sometimes is raised that the group is not contiguous from bonds.

Code to reproduce the behavior

This was first reported by @hejamu in Discord. He has provided me with a LAMMPS example structure for which this occurs, but I have since been able to reproduce the error with 1hvr.pdb, which is already in the tests (with the CONECT datafile reference).

import MDAnalysis as mda
from MDAnalysis.tests.datafiles import CONECT

u = mda.Universe(CONECT)
u.atoms.unwrap()

## ValueError: AtomGroup was not contiguous from bonds, process failed

Current version of MDAnalysis

2.0.0-dev0

Cause

A first inspection seems to indicate that unwrap is selecting the wrong set of atoms as a fragment to unwrap (in the 1hvr example only parts of the molecule have CONECT records). This is maybe related to the changes introduced in #3005, which touched this aspect of the code.

@germanbarcenas
Copy link

Hi! I've run into a similar issue before with my own work. It seems like the .pdb file bond connection is not being read. I had to fix by hardcoding some bonds to maintain pbc. Your issue might be similar to what I had experienced.

@hejamu
Copy link
Contributor

hejamu commented Feb 11, 2022

Isn't this issue fixed by #3353?

@mnmelo
Copy link
Member Author

mnmelo commented Jun 23, 2022

I'm closing this since, as @hejamu pointed out, the PR in #3353 fixes this (the PR message incorrectly states that it fixes #3353 instead of #3352).

@mnmelo mnmelo closed this as completed Jun 23, 2022
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

3 participants