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

Add unwrap to radius_of_gyration #2299

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package/CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ Enhancements
* added unwrap keyword to center_of_mass (PR #2286)
* added unwrap keyword to moment_of_inertia (PR #2287)
* added unwrap keyword to asphericity (PR #2290)
* added unwrap keyword to radius_of_gyration (PR #2299)

Changes
* added official support for Python 3.7 (PR #1963)
Expand Down
15 changes: 14 additions & 1 deletion package/MDAnalysis/core/topologyattrs.py
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,7 @@ def moment_of_inertia(group, **kwargs):
('moment_of_inertia', moment_of_inertia))

@warn_if_not_unique
@check_pbc_and_unwrap
def radius_of_gyration(group, **kwargs):
"""Radius of gyration.

Expand All @@ -945,6 +946,10 @@ def radius_of_gyration(group, **kwargs):
pbc : bool, optional
If ``True``, move all atoms within the primary unit cell before
calculation. [``False``]
unwrap : bool, optional
If ``True``, compounds will be unwrapped before computing their centers.
compound : {'group', 'segments', 'residues', 'molecules', 'fragments'}, optional
Which type of component to keep together during unwrapping.

Note
----
Expand All @@ -953,15 +958,23 @@ def radius_of_gyration(group, **kwargs):


.. versionchanged:: 0.8 Added *pbc* keyword
.. versionchanged:: 0.20.0 Added *unwrap* and *compound* parameter

"""
atomgroup = group.atoms
pbc = kwargs.pop('pbc', flags['use_pbc'])
masses = atomgroup.masses
unwrap = kwargs.pop('unwrap', False)
compound = kwargs.pop('compound', 'group')

com = atomgroup.center_of_mass(pbc=pbc, unwrap=unwrap, compound=compound)
if compound is not 'group':
com = (com * group.masses[:, None]).sum(axis=0) / group.masses.sum()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've added a compound parameter and not written tests for it, so this line never gets tested. What is this line doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it gets tested here

If the compound is 'residue', atomgroup.center_of_mass will return COM of each residue. This line takes the mean of each COM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@richardjgowers is this sufficient or should I add some more tests?


com = atomgroup.center_of_mass(pbc=pbc)
if pbc:
recenteredpos = atomgroup.pack_into_box(inplace=False) - com
elif unwrap:
recenteredpos = atomgroup.unwrap(compound=compound) - com
else:
recenteredpos = atomgroup.positions - com

Expand Down
8 changes: 8 additions & 0 deletions testsuite/MDAnalysisTests/core/test_atomgroup.py
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,7 @@ def ref_noUnwrap_residues(self):
[-211.8997285, 7059.07470427, -91.32156884],
[-721.50785456, -91.32156884, 6509.31735029]]),
'Asph': 0.02060121,
'ROG': 27.713009,
}

@pytest.fixture()
Expand All @@ -874,6 +875,7 @@ def ref_Unwrap_residues(self):
[-1330.489, 19315.253, 3306.212],
[ 2938.243, 3306.212, 8990.481]]),
'Asph': 0.2969491080,
'ROG': 40.686541,
}

@pytest.fixture()
Expand All @@ -886,6 +888,7 @@ def ref_noUnwrap(self):
[0.0, 98.6542, 0.0],
[0.0, 0.0, 98.65421327]]),
'Asph': 1.0,
'ROG': 0.9602093,
}

@pytest.fixture()
Expand All @@ -898,19 +901,22 @@ def ref_Unwrap(self):
[0.0, 132.673, 0.0],
[0.0, 0.0, 132.673]]),
'Asph': 1.0,
'ROG': 1.1135230,
}

def test_default_residues(self, ag, ref_noUnwrap_residues):
assert_almost_equal(ag.center_of_geometry(compound='residues'), ref_noUnwrap_residues['COG'], self.prec)
assert_almost_equal(ag.center_of_mass(compound='residues'), ref_noUnwrap_residues['COM'], self.prec)
assert_almost_equal(ag.moment_of_inertia(compound='residues'), ref_noUnwrap_residues['MOI'], self.prec)
assert_almost_equal(ag.asphericity(compound='residues'), ref_noUnwrap_residues['Asph'], self.prec)
assert_almost_equal(ag.radius_of_gyration(compound='residues'), ref_noUnwrap_residues['ROG'], self.prec)

def test_UnWrapFlag_residues(self, ag, ref_Unwrap_residues):
assert_almost_equal(ag.center_of_geometry(unwrap=True, compound='residues'), ref_Unwrap_residues['COG'], self.prec)
assert_almost_equal(ag.center_of_mass(unwrap=True, compound='residues'), ref_Unwrap_residues['COM'], self.prec)
assert_almost_equal(ag.moment_of_inertia(unwrap=True, compound='residues'), ref_Unwrap_residues['MOI'], self.prec)
assert_almost_equal(ag.asphericity(unwrap=True, compound='residues'), ref_Unwrap_residues['Asph'], self.prec)
assert_almost_equal(ag.radius_of_gyration(unwrap=True, compound='residues'), ref_Unwrap_residues['ROG'], self.prec)

def test_default(self, ref_noUnwrap):
u = UnWrapUniverse(is_triclinic=False)
Expand All @@ -922,6 +928,7 @@ def test_default(self, ref_noUnwrap):
assert_almost_equal(group.center_of_mass(), ref_noUnwrap['COM'], self.prec)
assert_almost_equal(group.moment_of_inertia(), ref_noUnwrap['MOI'], self.prec)
assert_almost_equal(group.asphericity(), ref_noUnwrap['Asph'], self.prec)
assert_almost_equal(group.radius_of_gyration(), ref_noUnwrap['ROG'], self.prec)

def test_UnWrapFlag(self, ref_Unwrap):
u = UnWrapUniverse(is_triclinic=False)
Expand All @@ -932,6 +939,7 @@ def test_UnWrapFlag(self, ref_Unwrap):
assert_almost_equal(group.center_of_mass(unwrap=True), ref_Unwrap['COM'], self.prec)
assert_almost_equal(group.moment_of_inertia(unwrap=True), ref_Unwrap['MOI'], self.prec)
assert_almost_equal(group.asphericity(unwrap=True), ref_Unwrap['Asph'], self.prec)
assert_almost_equal(group.radius_of_gyration(unwrap=True), ref_Unwrap['ROG'], self.prec)


class TestPBCFlag(object):
Expand Down