MDAnalysis / mdanalysis

MDAnalysis is a Python library to analyze molecular dynamics simulations.
https://mdanalysis.org
Other
1.29k stars 647 forks source link

Radius of Gyration needs wrap option #1760

Open kain88-de opened 6 years ago

kain88-de commented 6 years ago

Expected behaviour

Calculating correct radius of gyration when protein is split on the periodic boundary and we set pbc=True

Actual behaviour

Neither with the pbc option set or without it do I get the correct radius of gyration. This is because to correctly handle the split of a protein on periodic boundaries we would need to recombine proteins that are split (unwrap them)

Example protein for which the correct value should be 2-4 nm.

Code to reproduce the behaviour

import MDAnalysis as mda

u = mda.Universe('protein.gro')
u.atoms.radius_of_gyration(pbc=True)

Currently version of MDAnalysis:

(run python -c "import MDAnalysis as mda; print(mda.__version__)")

richardjgowers commented 6 years ago

The pbc keyword will apply PBC by forcing everything into the primary unit cell.

I think what you want is unwrap, to make the molecule whole and continuous, which is issue #1185 right?

kain88-de commented 6 years ago

yeah sorry, unwrap. Just forcing everything into the primary unit cell is definitely the wrong thing to do for the radius of gyration calculation.

orbeckst commented 6 years ago

Should we change what pbc=True does for radius_of_gyration()? Do we have enough information to always correctly unwrap the selection?

orbeckst commented 6 years ago

P.S.: I created the new label:PBC – we have so many issues relating to periodic boundary conditions – a GSOC student digging into this would be great.

richardjgowers commented 6 years ago

I'd rather pbc=True was consistent in all methods (ie packs into unit cell before calculation) but I agree that for Rgyr you actually want (the as yet not implemented) unwrap=True kwarg instead.

On Wed, 24 Jan 2018, 11:58 p.m. Oliver Beckstein, notifications@github.com wrote:

P.S.: I created the new label:PBC https://github.com/MDAnalysis/mdanalysis/labels/PBC – we have so many issues relating to periodic boundary conditions – a GSOC student digging into this would be great.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/MDAnalysis/mdanalysis/issues/1760#issuecomment-360314963, or mute the thread https://github.com/notifications/unsubscribe-auth/AI0jBwwLkp6ad5oSZ2lWSMZ4so2R7UM-ks5tN8ORgaJpZM4RrDPl .

orbeckst commented 6 years ago

Then rather have unwrap=True and ditch useless pbc keyword. Rather be clear than opaque (or somesuch Python Zen).

On Jan 24, 2018, at 5:28 PM, Richard Gowers notifications@github.com wrote:

I'd rather pbc=True was consistent in all methods (ie packs into unit cell before calculation) but I agree that for Rgyr you actually want (the as yet not implemented) unwrap=True kwarg instead.

-- Oliver Beckstein orbeckst@gmail.com * orbeckst@gmx.net

richardjgowers commented 6 years ago

I'm going to close this because it's a duplicate of #1185

kain88-de commented 6 years ago

I would like to keep this open as the pbc keyword doesn't produce a correct result right now as one would expect. The bug should be open until it's removed.