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.

Sign up for GitHub

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

Fix and cover RMSD #889

Merged
merged 6 commits into from
Jul 10, 2016
Merged

Fix and cover RMSD #889

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dfb3c99
Removed call to protected process_selection now that it isn't protected
jdetle Jul 6, 2016
a0c1bdb
Added tests for RMSD, added a testfile.
jdetle Jul 8, 2016
ea41a49
Tested saving
jdetle Jul 8, 2016
023408d
Test group selections
jdetle Jul 9, 2016
40eb01c
Fixed error messages
jdetle Jul 10, 2016
91ba82a
Asserted everywhere, fixed error messages.
jdetle Jul 10, 2016
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
10 changes: 5 additions & 5 deletions package/MDAnalysis/analysis/rms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@

import matplotlib.pyplot as plt
rmsd = R.rmsd.T # transpose makes it easier for plotting
time = rmsd[1]
time = rmsd[1]facebook.com
Copy link
Member

Choose a reason for hiding this comment

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

There's a random "facebook.com" in the docs. Please remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird...

fig = plt.figure(figsize=(4,4))
ax = fig.add_subplot(111)
ax.plot(time, rmsd[2], 'k-', label="all")
Expand Down Expand Up @@ -338,9 +338,9 @@ def __init__(self, traj, reference=None, select='all',
self.reference = self.universe
else:
self.reference = reference
self.select = _process_selection(select)
self.select = process_selection(select)
if groupselections is not None:
self.groupselections = [_process_selection(s) for s in groupselections]
self.groupselections = [process_selection(s) for s in groupselections]
else:
self.groupselections = []
self.mass_weighted = mass_weighted
Expand All @@ -351,7 +351,7 @@ def __init__(self, traj, reference=None, select='all',
self.ref_atoms = self.reference.select_atoms(*self.select['reference'])
self.traj_atoms = self.universe.select_atoms(*self.select['mobile'])
if len(self.ref_atoms) != len(self.traj_atoms):
logger.exception()
logger.exception('SelectionError: ref_atoms, traj_atoms unequal')
Copy link
Member

Choose a reason for hiding this comment

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

(I think you can also leave the message for logger.exception empty and it will show the stack trace.) But otherwise make sure that they have the same message, e.g.

errmsg = "Reference and trajectory atom ... N_ref={0:d}, N_traj={1:d}".format(...)
logger.exception(errmsg)
raise SelectionError(errmsg)

raise SelectionError("Reference and trajectory atom selections do "
"not contain the same number of atoms: "
"N_ref={0:d}, N_traj={1:d}".format(
Expand Down Expand Up @@ -386,7 +386,7 @@ def __init__(self, traj, reference=None, select='all',
for igroup, (sel, atoms) in enumerate(zip(self.groupselections,
self.groupselections_atoms)):
if len(atoms['mobile']) != len(atoms['reference']):
logger.exception()
logger.exception('SelectionError: Group Selection')
Copy link
Member

Choose a reason for hiding this comment

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

see above about logging/raising errors

raise SelectionError(
"Group selection {0}: {1} | {2}: Reference and trajectory "
"atom selections do not contain the same number of atoms: "
Expand Down
67 changes: 66 additions & 1 deletion testsuite/MDAnalysisTests/analysis/test_rms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@

import os

from MDAnalysis.exceptions import SelectionError, NoDataError
from MDAnalysisTests.datafiles import GRO, XTC, rmsfArray, PSF, DCD
from MDAnalysisTests import tempdir


class TestRMSD(object):
class Testrmsd(object):
def __init__(self):
shape = (5, 3)
# vectors with length one
Expand Down Expand Up @@ -128,6 +129,70 @@ def test_with_superposition_equal(self):
rmsd_superposition = rms.rmsd(A, B, center=True, superposition=True)
assert_almost_equal(rmsd, rmsd_superposition)

class TestRMSD(object):
Copy link
Member

Choose a reason for hiding this comment

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

Thank you!!

def setUp(self):
self.universe = MDAnalysis.Universe(PSF, DCD)
self.tempdir = tempdir.TempDir()
self.outfile = os.path.join(self.tempdir.name, 'rmsd.npy')

def tearDown(self):
del self.universe
del self.tempdir

def test_rmsd(self):
RMSD = MDAnalysis.analysis.rms.RMSD(self.universe, select='name CA')
RMSD.run(step=49)
correct_values = [[ 0, 0, 0],
Copy link
Member

Choose a reason for hiding this comment

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

store the correct_values in the class (a "fixture") so that you can refer to them here and in test_mass_weighted_and_save().

[ 49, 48.9999, 4.68953]]
assert_almost_equal(RMSD.rmsd, correct_values, 4,
err_msg="error: rmsd profile should match test " +
"values")


def test_rmsd_single_frame(self):
RMSD = MDAnalysis.analysis.rms.RMSD(self.universe, select='name CA')
RMSD.run(start=5, stop=6)
Copy link
Member

@orbeckst orbeckst Jul 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

assert something

A quick way to get the values is just to put in an assert with harbage values, run the test, note the real values, replace garbage values with real values and you're done.



def test_mass_weighted_and_save(self):
RMSD = MDAnalysis.analysis.rms.RMSD(self.universe, select='name CA')
RMSD.run(step=49, mass_weighted= True)
RMSD.save()
Copy link
Member

Choose a reason for hiding this comment

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

read the save file and compare to the correct_values (see above) to assert correct file writing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay thanks for the review. This is stuff I can get done in some free time
tonight. I am sitting in my car about to drive up to the Bay Area as of
this very moment.
On Sat, Jul 9, 2016 at 3:16 PM Oliver Beckstein [email protected]
wrote:

In testsuite/MDAnalysisTests/analysis/test_rms.py
#889 (comment):

  •    correct_values = [[  0, 0, 0],

  •                      [  49, 48.9999, 4.68953]]

  •    assert_almost_equal(RMSD.rmsd, correct_values, 4,

  •                        err_msg="error: rmsd profile should match test " +

  •                        "values")
  • def test_rmsd_single_frame(self):
  •    RMSD = MDAnalysis.analysis.rms.RMSD(self.universe, select='name CA')

  •    RMSD.run(start=5, stop=6)
  • def test_mass_weighted_and_save(self):
  •    RMSD = MDAnalysis.analysis.rms.RMSD(self.universe, select='name CA')

  •    RMSD.run(step=49, mass_weighted= True)

  •    RMSD.save()

read the save file and compare to the correct_values (see above) to assert
correct file writing.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/MDAnalysis/mdanalysis/pull/889/files/023408db07dbe5fcb0bc71d0298fbb0b2dc11d88#r70172708,
or mute the thread
https://github.com/notifications/unsubscribe/AKcARr1QEcytOeSe-fTciWayQypUT6wPks5qUB3IgaJpZM4JGYJ2
.

Have a wonderful day,
John J. Detlefs


def test_rmsd_group_selections(self):
RMSD = MDAnalysis.analysis.rms.RMSD(self.universe,
groupselections=
['backbone','name CA'])
RMSD.run(step=49)
Copy link
Member

Choose a reason for hiding this comment

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

assert something



Copy link
Member

Choose a reason for hiding this comment

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

excellent job testing the failure cases!

@raises(SelectionError)
def test_ref_length_unequal_len(self):
reference = MDAnalysis.Universe(PSF, DCD)
reference.atoms = reference.atoms[:-1]
RMSD = MDAnalysis.analysis.rms.RMSD(self.universe,
reference=reference)

@raises(SelectionError)
def test_mass_mismatches(self):
reference = MDAnalysis.Universe(PSF, DCD)
reference.atoms.masses = 10
RMSD = MDAnalysis.analysis.rms.RMSD(self.universe,
reference=reference)


@raises(SelectionError)
def test_group_selections_unequal_len(self):
reference = MDAnalysis.Universe(PSF, DCD)
reference.atoms[0].resname='NOTMET'
RMSD = MDAnalysis.analysis.rms.RMSD(self.universe,
reference=reference,
groupselections=
['resname MET','type NH3'])
@raises(NoDataError)
def test_save_before_run(self):
RMSD = MDAnalysis.analysis.rms.RMSD(self.universe)
RMSD.save('blah')

class TestRMSF(TestCase):
def setUp(self):
Expand Down