From 197660a90657fbfa03c2716722a5b5dd60f418b3 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 Feb 2020 14:29:06 +0000 Subject: [PATCH 1/8] added minimum hydrogen mass --- .../analysis/hydrogenbonds/hbond_analysis.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py index b5d0eb5a3fe..36f385ce19d 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py @@ -169,7 +169,7 @@ """ from __future__ import absolute_import, division -import numpy as np +import numpy as np from .. import base from MDAnalysis.lib.distances import capped_distance, calc_angles @@ -234,7 +234,12 @@ def __init__(self, universe, donors_sel=None, hydrogens_sel=None, acceptors_sel= self.d_h_a_angle = d_h_a_angle_cutoff self.update_selections = update_selections - def guess_hydrogens(self, selection='all', max_mass=1.1, min_charge=0.3): + def guess_hydrogens(self, + selection='all', + max_mass=1.1, + min_charge=0.3, + min_mass=0.9 + ): """Guesses which hydrogen atoms should be used in the analysis. Parameters @@ -263,14 +268,20 @@ def guess_hydrogens(self, selection='all', max_mass=1.1, min_charge=0.3): Alternatively, this function may be used to quickly generate a :class:`str` of potential hydrogen atoms involved in hydrogen bonding. This str may then be modified before being used to set the attribute :attr:`hydrogens_sel`. + + .. versionchanged: 1.0.0 + Added `min_mass` parameter to specify minimum mass (Issue #2472) """ + assert min_mass < max_mass + ag = self.u.select_atoms(selection) hydrogens_ag = ag[ - np.logical_and( + np.logical_and.reduce(( + ag.masses > min_mass, ag.masses < max_mass, ag.charges > min_charge - ) + )) ] hydrogens_list = np.unique( From ef8e8fffa3bc9528700e441401b32b7c0bcaa965 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 Feb 2020 14:29:31 +0000 Subject: [PATCH 2/8] added tests for guess_hydrogens --- .../analysis/test_hydrogenbonds_analysis.py | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py b/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py index b4b1ecb5c2a..d3b8e728fbe 100644 --- a/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py +++ b/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py @@ -139,7 +139,56 @@ def test_guess_donors(self, h): ref_donors = "(resname TIP3 and name OH2)" donors = h.guess_donors(selection='all', max_charge=-0.5) assert donors == ref_donors +class TestHydrogenBondAnalysisTIP3P_GuessHyrdogens_NoTopology(object): + """ + Guess the hydrogen atoms involved in hydrogen bonds using the mass and + partial charge of the atoms. + """ + + @staticmethod + @pytest.fixture(scope='class') + def universe(): + return MDAnalysis.Universe(waterPSF, waterDCD) + + kwargs = { + 'donors_sel': None, + 'hydrogens_sel': None, + 'acceptors_sel': None, + 'd_h_cutoff': 1.2, + 'd_a_cutoff': 3.0, + 'd_h_a_angle_cutoff': 120.0 + } + + @pytest.fixture(scope='class') + def h(self, universe): + h = HydrogenBondAnalysis(universe, **self.kwargs) + return h + + def test_guess_hydrogens(self, h): + + ref_hydrogens = "(resname TIP3 and name H1) or (resname TIP3 and name H2)" + hydrogens = h.guess_hydrogens(selection='all') + assert hydrogens == ref_hydrogens + + def test_guess_hydrogens_min_mass(self, h): + + hydrogens = h.guess_hydrogens(selection='all', min_mass=1.05) + assert hydrogens == "" + + def test_guess_hydrogens_max_mass(self, h): + + hydrogens = h.guess_hydrogens(selection='all', max_mass=0.95) + assert hydrogens == "" + + def test_guess_hydrogens_min_charge(self, h): + + hydrogens = h.guess_hydrogens(selection='all', min_charge=1.0) + assert hydrogens == "" + + def test_guess_hydrogens_min_max_mass(self, h): + with pytest.raises(AssertionError): + hydrogens = h.guess_hydrogens(selection='all', min_mass=1.1, max_mass=0.9) class TestHydrogenBondAnalysisTIP3PStartStep(object): """Uses the same distance and cutoff hydrogen bond criteria as :class:`TestHydrogenBondAnalysisTIP3P` but starting From 48dbfae35a867c5f7f52819cf028c07140928b88 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 Feb 2020 14:31:05 +0000 Subject: [PATCH 3/8] changelog --- package/CHANGELOG | 2 ++ 1 file changed, 2 insertions(+) diff --git a/package/CHANGELOG b/package/CHANGELOG index 20f00667a66..8d49c5d0a35 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -81,6 +81,8 @@ Enhancements convert between a parmed.Structure and MDAnalysis Universe (PR #2404) Changes + * Added `min_mass` parameter to `guess_hydrogens` function in `HydrogenBondAnalysis` + set to 0.9 by default (Issue #2472) * Removes `save()` function from contacts, diffusionmap, hole, LinearDensity, and rms (Issue #1745). * Removes; `save_table()` from :class:`HydrogenBondAnalysis`, From b570297c867bac93b00bc6d9030c50670f398b43 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 Feb 2020 16:55:20 +0000 Subject: [PATCH 4/8] remove assertion from code --- package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py | 3 ++- .../MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py index 36f385ce19d..5927a78f28d 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py @@ -273,7 +273,8 @@ def guess_hydrogens(self, Added `min_mass` parameter to specify minimum mass (Issue #2472) """ - assert min_mass < max_mass + if min_mass > max_mass: + raise ValueError("min_mass is higher than (or equal to) max_mass") ag = self.u.select_atoms(selection) hydrogens_ag = ag[ diff --git a/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py b/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py index d3b8e728fbe..97178020035 100644 --- a/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py +++ b/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py @@ -139,7 +139,7 @@ def test_guess_donors(self, h): ref_donors = "(resname TIP3 and name OH2)" donors = h.guess_donors(selection='all', max_charge=-0.5) assert donors == ref_donors -class TestHydrogenBondAnalysisTIP3P_GuessHyrdogens_NoTopology(object): +class TestHydrogenBondAnalysisTIP3P_GuessHydrogens_NoTopology(object): """ Guess the hydrogen atoms involved in hydrogen bonds using the mass and partial charge of the atoms. @@ -187,7 +187,7 @@ def test_guess_hydrogens_min_charge(self, h): def test_guess_hydrogens_min_max_mass(self, h): - with pytest.raises(AssertionError): + with pytest.raises(ValueError): hydrogens = h.guess_hydrogens(selection='all', min_mass=1.1, max_mass=0.9) class TestHydrogenBondAnalysisTIP3PStartStep(object): From 4417ab04ab73028d7142bc00e975eb7839796a5c Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 Feb 2020 17:28:26 +0000 Subject: [PATCH 5/8] check exception message --- .../MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py b/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py index 97178020035..4968b5d41dc 100644 --- a/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py +++ b/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py @@ -187,9 +187,13 @@ def test_guess_hydrogens_min_charge(self, h): def test_guess_hydrogens_min_max_mass(self, h): - with pytest.raises(ValueError): + with pytest.raises(ValueError) as err: + hydrogens = h.guess_hydrogens(selection='all', min_mass=1.1, max_mass=0.9) + assert err.value.args[0] == "min_mass is higher than (or equal to) max_mass" + + class TestHydrogenBondAnalysisTIP3PStartStep(object): """Uses the same distance and cutoff hydrogen bond criteria as :class:`TestHydrogenBondAnalysisTIP3P` but starting with the second frame and using every other frame in the analysis. From 46f8a1cf437b2c7c1ea6e5af4d99b20db5cdcac5 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Mon, 10 Feb 2020 09:41:00 +0000 Subject: [PATCH 6/8] put tests together and fix linter warnings --- .../analysis/test_hydrogenbonds_analysis.py | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py b/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py index 4968b5d41dc..ac3118ee222 100644 --- a/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py +++ b/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py @@ -57,7 +57,7 @@ def h(self, universe): def test_hbond_analysis(self, h): - assert len(np.unique(h.hbonds[:,0])) == 10 + assert len(np.unique(h.hbonds[:, 0])) == 10 assert len(h.hbonds) == 32 reference = { @@ -98,6 +98,7 @@ def test_count_by_ids(self, h): assert_array_equal(counts, ref_counts) + class TestHydrogenBondAnalysisTIP3P_GuessAcceptors_GuessHydrogens_UseTopology_(TestHydrogenBondAnalysisTIP3P): """Uses the same distance and cutoff hydrogen bond criteria as :class:`TestHydrogenBondAnalysisTIP3P`, so the results are identical, but the hydrogens and acceptors are guessed whilst the donor-hydrogen pairs are determined @@ -111,6 +112,7 @@ class TestHydrogenBondAnalysisTIP3P_GuessAcceptors_GuessHydrogens_UseTopology_(T 'd_h_a_angle_cutoff': 120.0 } + class TestHydrogenBondAnalysisTIP3P_GuessDonors_NoTopology(object): """Guess the donor atoms involved in hydrogen bonds using the partial charges of the atoms. """ @@ -139,6 +141,8 @@ def test_guess_donors(self, h): ref_donors = "(resname TIP3 and name OH2)" donors = h.guess_donors(selection='all', max_charge=-0.5) assert donors == ref_donors + + class TestHydrogenBondAnalysisTIP3P_GuessHydrogens_NoTopology(object): """ Guess the hydrogen atoms involved in hydrogen bonds using the mass and @@ -170,17 +174,15 @@ def test_guess_hydrogens(self, h): hydrogens = h.guess_hydrogens(selection='all') assert hydrogens == ref_hydrogens - def test_guess_hydrogens_min_mass(self, h): - - hydrogens = h.guess_hydrogens(selection='all', min_mass=1.05) - assert hydrogens == "" - - def test_guess_hydrogens_max_mass(self, h): - - hydrogens = h.guess_hydrogens(selection='all', max_mass=0.95) - assert hydrogens == "" - - def test_guess_hydrogens_min_charge(self, h): + pytest.mark.parametrize( + "min_mass, max_mass, min_charge", + [ + (1.05, 1.10, 0.30), + (0.90, 0.95, 0.30), + (0.90, 1.10, 1.00) + ] + ) + def test_guess_hydrogens_empty_selection(self, h): hydrogens = h.guess_hydrogens(selection='all', min_charge=1.0) assert hydrogens == "" @@ -189,7 +191,7 @@ def test_guess_hydrogens_min_max_mass(self, h): with pytest.raises(ValueError) as err: - hydrogens = h.guess_hydrogens(selection='all', min_mass=1.1, max_mass=0.9) + h.guess_hydrogens(selection='all', min_mass=1.1, max_mass=0.9) assert err.value.args[0] == "min_mass is higher than (or equal to) max_mass" @@ -221,7 +223,7 @@ def h(self, universe): def test_hbond_analysis(self, h): - assert len(np.unique(h.hbonds[:,0])) == 5 + assert len(np.unique(h.hbonds[:, 0])) == 5 assert len(h.hbonds) == 15 reference = { From 1e5e4b14f693050961ae5bcb4f1858cf03a23a39 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Mon, 10 Feb 2020 13:40:08 +0000 Subject: [PATCH 7/8] use pytest match --- .../analysis/test_hydrogenbonds_analysis.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py b/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py index ac3118ee222..451b3590cf8 100644 --- a/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py +++ b/testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py @@ -189,12 +189,11 @@ def test_guess_hydrogens_empty_selection(self, h): def test_guess_hydrogens_min_max_mass(self, h): - with pytest.raises(ValueError) as err: + errmsg = "min_mass is higher than \(or equal to\) max_mass" - h.guess_hydrogens(selection='all', min_mass=1.1, max_mass=0.9) - - assert err.value.args[0] == "min_mass is higher than (or equal to) max_mass" + with pytest.raises(ValueError, match=errmsg): + h.guess_hydrogens(selection='all', min_mass=1.1, max_mass=0.9) class TestHydrogenBondAnalysisTIP3PStartStep(object): """Uses the same distance and cutoff hydrogen bond criteria as :class:`TestHydrogenBondAnalysisTIP3P` but starting From 23cc898510779022487242fcf2b65f68c0012bda Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Tue, 11 Feb 2020 10:28:48 +0000 Subject: [PATCH 8/8] put min_mass check at the end --- package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py index 5927a78f28d..aa8fca2c5c7 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py @@ -279,9 +279,9 @@ def guess_hydrogens(self, ag = self.u.select_atoms(selection) hydrogens_ag = ag[ np.logical_and.reduce(( - ag.masses > min_mass, ag.masses < max_mass, - ag.charges > min_charge + ag.charges > min_charge, + ag.masses > min_mass, )) ]