From aa306179bbd35219ee466a00271501bbffe16678 Mon Sep 17 00:00:00 2001 From: orionarcher Date: Thu, 1 Jun 2023 14:05:03 -0700 Subject: [PATCH 01/12] add an ignore_same_residue kwarg to InterRDF and test it --- package/MDAnalysis/analysis/rdf.py | 11 +++++++++++ testsuite/MDAnalysisTests/analysis/test_rdf.py | 6 ++++++ 2 files changed, 17 insertions(+) diff --git a/package/MDAnalysis/analysis/rdf.py b/package/MDAnalysis/analysis/rdf.py index 452f0a74189..780e1552aab 100644 --- a/package/MDAnalysis/analysis/rdf.py +++ b/package/MDAnalysis/analysis/rdf.py @@ -125,6 +125,8 @@ class InterRDF(AnalysisBase): exclusion_block : tuple A tuple representing the tile to exclude from the distance array. + ignore_same_residue : bool + If `True`, ignore distances between atoms in the same residue. verbose : bool Show detailed progress of the calculation if set to `True` @@ -220,6 +222,7 @@ def __init__(self, range=(0.0, 15.0), norm="rdf", exclusion_block=None, + ignore_same_residue=False, **kwargs): super(InterRDF, self).__init__(g1.universe.trajectory, **kwargs) self.g1 = g1 @@ -229,6 +232,7 @@ def __init__(self, self.rdf_settings = {'bins': nbins, 'range': range} self._exclusion_block = exclusion_block + self.ignore_same_residue = ignore_same_residue if self.norm not in ['rdf', 'density', 'none']: raise ValueError(f"'{self.norm}' is an invalid norm. " @@ -261,6 +265,13 @@ def _single_frame(self): mask = np.where(idxA != idxB)[0] dist = dist[mask] + if self.ignore_same_residue: + # Ignore distances between atoms in the same residue + resixA = self.g1.resids[pairs[:, 0]] + resixB = self.g2.resids[pairs[:, 1]] + mask = np.where(resixA != resixB)[0] + dist = dist[mask] + count, _ = np.histogram(dist, **self.rdf_settings) self.results.count += count diff --git a/testsuite/MDAnalysisTests/analysis/test_rdf.py b/testsuite/MDAnalysisTests/analysis/test_rdf.py index 3717ea1898b..e75802bcc41 100644 --- a/testsuite/MDAnalysisTests/analysis/test_rdf.py +++ b/testsuite/MDAnalysisTests/analysis/test_rdf.py @@ -93,6 +93,12 @@ def test_exclusion(sels): rdf = InterRDF(s1, s2, exclusion_block=(1, 2)).run() assert rdf.results.count.sum() == 4 +def test_ignore_same_residues(sels): + # should see two distances with 4 counts each + s1, s2 = sels + rdf = InterRDF(s2, s2, ignore_same_residue=True).run() + assert rdf.rdf[0] == 0 + assert rdf.results.count.sum() == 8 @pytest.mark.parametrize("attr", ("rdf", "bins", "edges", "count")) def test_rdf_attr_warning(sels, attr): From 782e07d5755693a7dce78f7450caa15d1e258b6e Mon Sep 17 00:00:00 2001 From: orionarcher Date: Thu, 1 Jun 2023 14:13:37 -0700 Subject: [PATCH 02/12] update changelog --- package/CHANGELOG | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 93ca9323533..fc399110d2f 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -13,13 +13,14 @@ The rules for this file: * release numbers follow "Semantic Versioning" http://semver.org ------------------------------------------------------------------------------ -??/??/?? IAlibay +??/??/?? IAlibay, orionarcher * 2.6.0 Fixes Enhancements + * Added a ignore_same_residue kwarg to InterRDF (PR #4161) Changes From 14ab59ae459fe744896b5012f019930fefc8981a Mon Sep 17 00:00:00 2001 From: orionarcher Date: Thu, 1 Jun 2023 14:55:56 -0700 Subject: [PATCH 03/12] change resid to resindices --- package/MDAnalysis/analysis/rdf.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package/MDAnalysis/analysis/rdf.py b/package/MDAnalysis/analysis/rdf.py index 780e1552aab..654211972d0 100644 --- a/package/MDAnalysis/analysis/rdf.py +++ b/package/MDAnalysis/analysis/rdf.py @@ -267,8 +267,8 @@ def _single_frame(self): if self.ignore_same_residue: # Ignore distances between atoms in the same residue - resixA = self.g1.resids[pairs[:, 0]] - resixB = self.g2.resids[pairs[:, 1]] + resixA = self.g1.resindices[pairs[:, 0]] + resixB = self.g2.resindices[pairs[:, 1]] mask = np.where(resixA != resixB)[0] dist = dist[mask] From 89370924c4b15ddd41e24b4d3dec9c83e92c7d97 Mon Sep 17 00:00:00 2001 From: orionarcher Date: Mon, 5 Jun 2023 13:30:38 -0700 Subject: [PATCH 04/12] allow residues, segments, or chains to be excluded --- package/MDAnalysis/analysis/rdf.py | 17 ++++++++++++----- testsuite/MDAnalysisTests/analysis/test_rdf.py | 2 +- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/package/MDAnalysis/analysis/rdf.py b/package/MDAnalysis/analysis/rdf.py index 654211972d0..3662ad5802b 100644 --- a/package/MDAnalysis/analysis/rdf.py +++ b/package/MDAnalysis/analysis/rdf.py @@ -125,8 +125,10 @@ class InterRDF(AnalysisBase): exclusion_block : tuple A tuple representing the tile to exclude from the distance array. - ignore_same_residue : bool - If `True`, ignore distances between atoms in the same residue. + exclude_same : str + Will exclude pairs of atoms that share the same "residue", "segment", or "chain". + Those are the only valid values. This is intended to remove atoms that are + spatially correlated due to direct bonded connections. verbose : bool Show detailed progress of the calculation if set to `True` @@ -222,7 +224,7 @@ def __init__(self, range=(0.0, 15.0), norm="rdf", exclusion_block=None, - ignore_same_residue=False, + exclude_same=None, **kwargs): super(InterRDF, self).__init__(g1.universe.trajectory, **kwargs) self.g1 = g1 @@ -232,7 +234,12 @@ def __init__(self, self.rdf_settings = {'bins': nbins, 'range': range} self._exclusion_block = exclusion_block - self.ignore_same_residue = ignore_same_residue + assert exclude_same is None or exclude_same in ['residue', 'segment', 'chain'], ( + "The exclude_same argument to InterRDF must be None, 'residue', 'segment' " + "or 'chain'." + ) + name_to_attr = {'residue': 'resindices', 'segment': 'segindices', 'chain': 'chainids'} + self.exclude_same = name_to_attr.get(exclude_same) if self.norm not in ['rdf', 'density', 'none']: raise ValueError(f"'{self.norm}' is an invalid norm. " @@ -265,7 +272,7 @@ def _single_frame(self): mask = np.where(idxA != idxB)[0] dist = dist[mask] - if self.ignore_same_residue: + if self.exclude_same: # Ignore distances between atoms in the same residue resixA = self.g1.resindices[pairs[:, 0]] resixB = self.g2.resindices[pairs[:, 1]] diff --git a/testsuite/MDAnalysisTests/analysis/test_rdf.py b/testsuite/MDAnalysisTests/analysis/test_rdf.py index e75802bcc41..8e3e378ad86 100644 --- a/testsuite/MDAnalysisTests/analysis/test_rdf.py +++ b/testsuite/MDAnalysisTests/analysis/test_rdf.py @@ -96,7 +96,7 @@ def test_exclusion(sels): def test_ignore_same_residues(sels): # should see two distances with 4 counts each s1, s2 = sels - rdf = InterRDF(s2, s2, ignore_same_residue=True).run() + rdf = InterRDF(s2, s2, exclude_same="residue").run() assert rdf.rdf[0] == 0 assert rdf.results.count.sum() == 8 From 0d788b70352e0a860289a31b35ae56b30223dba7 Mon Sep 17 00:00:00 2001 From: orionarcher Date: Wed, 21 Jun 2023 17:51:11 -0700 Subject: [PATCH 05/12] fix to allow all attr and change assert to raise --- package/MDAnalysis/analysis/rdf.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/package/MDAnalysis/analysis/rdf.py b/package/MDAnalysis/analysis/rdf.py index 3662ad5802b..195a8272c78 100644 --- a/package/MDAnalysis/analysis/rdf.py +++ b/package/MDAnalysis/analysis/rdf.py @@ -234,10 +234,11 @@ def __init__(self, self.rdf_settings = {'bins': nbins, 'range': range} self._exclusion_block = exclusion_block - assert exclude_same is None or exclude_same in ['residue', 'segment', 'chain'], ( - "The exclude_same argument to InterRDF must be None, 'residue', 'segment' " - "or 'chain'." - ) + if exclude_same is not None and exclude_same not in ['residue', 'segment', 'chain']: + raise ValueError( + "The exclude_same argument to InterRDF must be None, 'residue', 'segment' " + "or 'chain'." + ) name_to_attr = {'residue': 'resindices', 'segment': 'segindices', 'chain': 'chainids'} self.exclude_same = name_to_attr.get(exclude_same) @@ -274,8 +275,8 @@ def _single_frame(self): if self.exclude_same: # Ignore distances between atoms in the same residue - resixA = self.g1.resindices[pairs[:, 0]] - resixB = self.g2.resindices[pairs[:, 1]] + resixA = getattr(self.g1, self.exclude_same)[pairs[:, 0]] + resixB = getattr(self.g2, self.exclude_same)[pairs[:, 1]] mask = np.where(resixA != resixB)[0] dist = dist[mask] From 75cae86c6314dbaf08f0c83fc4753d38d55d0765 Mon Sep 17 00:00:00 2001 From: orionarcher Date: Wed, 5 Jul 2023 17:04:28 -0700 Subject: [PATCH 06/12] address comments from hmacdope and RMeli --- package/CHANGELOG | 2 +- package/MDAnalysis/analysis/rdf.py | 16 ++++++++++------ testsuite/MDAnalysisTests/analysis/test_rdf.py | 13 +++++++++++-- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index fc399110d2f..a88eafa0bb2 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -20,7 +20,7 @@ The rules for this file: Fixes Enhancements - * Added a ignore_same_residue kwarg to InterRDF (PR #4161) + * Added a `exclude_same` kwarg to InterRDF (PR #4161) Changes diff --git a/package/MDAnalysis/analysis/rdf.py b/package/MDAnalysis/analysis/rdf.py index 195a8272c78..0480c2adf13 100644 --- a/package/MDAnalysis/analysis/rdf.py +++ b/package/MDAnalysis/analysis/rdf.py @@ -239,7 +239,11 @@ def __init__(self, "The exclude_same argument to InterRDF must be None, 'residue', 'segment' " "or 'chain'." ) - name_to_attr = {'residue': 'resindices', 'segment': 'segindices', 'chain': 'chainids'} + if exclude_same is not None and exclusion_block is not None: + raise ValueError( + "The exclude_same argument to InterRDF cannot be used with exclusion_block." + ) + name_to_attr = {'residue': 'resindices', 'segment': 'segindices', 'chain': 'chainIDs'} self.exclude_same = name_to_attr.get(exclude_same) if self.norm not in ['rdf', 'density', 'none']: @@ -273,11 +277,11 @@ def _single_frame(self): mask = np.where(idxA != idxB)[0] dist = dist[mask] - if self.exclude_same: - # Ignore distances between atoms in the same residue - resixA = getattr(self.g1, self.exclude_same)[pairs[:, 0]] - resixB = getattr(self.g2, self.exclude_same)[pairs[:, 1]] - mask = np.where(resixA != resixB)[0] + if self.exclude_same is not None: + # Ignore distances between atoms in the same attribute + attr_ix_a = getattr(self.g1, self.exclude_same)[pairs[:, 0]] + attr_ix_b = getattr(self.g2, self.exclude_same)[pairs[:, 1]] + mask = np.where(attr_ix_a != attr_ix_b)[0] dist = dist[mask] count, _ = np.histogram(dist, **self.rdf_settings) diff --git a/testsuite/MDAnalysisTests/analysis/test_rdf.py b/testsuite/MDAnalysisTests/analysis/test_rdf.py index 8e3e378ad86..e05414527bb 100644 --- a/testsuite/MDAnalysisTests/analysis/test_rdf.py +++ b/testsuite/MDAnalysisTests/analysis/test_rdf.py @@ -93,13 +93,22 @@ def test_exclusion(sels): rdf = InterRDF(s1, s2, exclusion_block=(1, 2)).run() assert rdf.results.count.sum() == 4 -def test_ignore_same_residues(sels): + +@pytest.mark.parametrize("attr", ("residue")) +def test_ignore_same_residues(sels, attr): # should see two distances with 4 counts each s1, s2 = sels - rdf = InterRDF(s2, s2, exclude_same="residue").run() + rdf = InterRDF(s2, s2, exclude_same=attr).run() assert rdf.rdf[0] == 0 assert rdf.results.count.sum() == 8 + +def test_ignore_same_residues_fails(sels): + # should see two distances with 4 counts each + s1, s2 = sels + with pytest.raises(ValueError): + rdf = InterRDF(s2, s2, exclude_same="unsupported").run() + @pytest.mark.parametrize("attr", ("rdf", "bins", "edges", "count")) def test_rdf_attr_warning(sels, attr): s1, s2 = sels From 1bd8348acf854f1f000de6147f29c3bf0400451c Mon Sep 17 00:00:00 2001 From: Orion Cohen <27712051+orionarcher@users.noreply.github.com> Date: Wed, 12 Jul 2023 14:59:26 -0700 Subject: [PATCH 07/12] Update testsuite/MDAnalysisTests/analysis/test_rdf.py Co-authored-by: Rocco Meli --- testsuite/MDAnalysisTests/analysis/test_rdf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsuite/MDAnalysisTests/analysis/test_rdf.py b/testsuite/MDAnalysisTests/analysis/test_rdf.py index e05414527bb..2b78e9ab912 100644 --- a/testsuite/MDAnalysisTests/analysis/test_rdf.py +++ b/testsuite/MDAnalysisTests/analysis/test_rdf.py @@ -106,7 +106,7 @@ def test_ignore_same_residues(sels, attr): def test_ignore_same_residues_fails(sels): # should see two distances with 4 counts each s1, s2 = sels - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="The exclude_same argument to InterRDF must be"): rdf = InterRDF(s2, s2, exclude_same="unsupported").run() @pytest.mark.parametrize("attr", ("rdf", "bins", "edges", "count")) From 21d6a02fb64d9840067c8a4ced6606cf1357de7e Mon Sep 17 00:00:00 2001 From: Orion Cohen <27712051+orionarcher@users.noreply.github.com> Date: Wed, 12 Jul 2023 15:14:57 -0700 Subject: [PATCH 08/12] Update testsuite/MDAnalysisTests/analysis/test_rdf.py Co-authored-by: Rocco Meli --- testsuite/MDAnalysisTests/analysis/test_rdf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsuite/MDAnalysisTests/analysis/test_rdf.py b/testsuite/MDAnalysisTests/analysis/test_rdf.py index 2b78e9ab912..c54ed02179d 100644 --- a/testsuite/MDAnalysisTests/analysis/test_rdf.py +++ b/testsuite/MDAnalysisTests/analysis/test_rdf.py @@ -94,7 +94,7 @@ def test_exclusion(sels): assert rdf.results.count.sum() == 4 -@pytest.mark.parametrize("attr", ("residue")) +@pytest.mark.parametrize("attr", ("residue",)) def test_ignore_same_residues(sels, attr): # should see two distances with 4 counts each s1, s2 = sels From d003acdc580f065c555ec5b58347d0b6c7fc679a Mon Sep 17 00:00:00 2001 From: orionarcher Date: Wed, 12 Jul 2023 16:02:16 -0700 Subject: [PATCH 09/12] add tests for segment and chain in test_ignore_same_residues --- testsuite/MDAnalysisTests/analysis/test_rdf.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/testsuite/MDAnalysisTests/analysis/test_rdf.py b/testsuite/MDAnalysisTests/analysis/test_rdf.py index c54ed02179d..04c9f1f8624 100644 --- a/testsuite/MDAnalysisTests/analysis/test_rdf.py +++ b/testsuite/MDAnalysisTests/analysis/test_rdf.py @@ -32,7 +32,9 @@ @pytest.fixture() def u(): - return mda.Universe(two_water_gro, in_memory=True) + u = mda.Universe(two_water_gro, in_memory=True) + u.add_TopologyAttr('chainIDs', u.atoms.resids) + return u @pytest.fixture() @@ -94,13 +96,16 @@ def test_exclusion(sels): assert rdf.results.count.sum() == 4 -@pytest.mark.parametrize("attr", ("residue",)) -def test_ignore_same_residues(sels, attr): +@pytest.mark.parametrize("attr, count", [ + ("residue", 8), + ("segment", 0), + ("chain", 8)]) +def test_ignore_same_residues(sels, attr, count): # should see two distances with 4 counts each s1, s2 = sels rdf = InterRDF(s2, s2, exclude_same=attr).run() assert rdf.rdf[0] == 0 - assert rdf.results.count.sum() == 8 + assert rdf.results.count.sum() == count def test_ignore_same_residues_fails(sels): From f5c4a7b4a7b1c0c98b38a5c264d2c5d82c4f38ba Mon Sep 17 00:00:00 2001 From: Orion Cohen <27712051+orionarcher@users.noreply.github.com> Date: Thu, 13 Jul 2023 16:53:44 -0700 Subject: [PATCH 10/12] Update testsuite/MDAnalysisTests/analysis/test_rdf.py Co-authored-by: Rocco Meli --- testsuite/MDAnalysisTests/analysis/test_rdf.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/testsuite/MDAnalysisTests/analysis/test_rdf.py b/testsuite/MDAnalysisTests/analysis/test_rdf.py index 04c9f1f8624..e440d47aaa0 100644 --- a/testsuite/MDAnalysisTests/analysis/test_rdf.py +++ b/testsuite/MDAnalysisTests/analysis/test_rdf.py @@ -114,6 +114,10 @@ def test_ignore_same_residues_fails(sels): with pytest.raises(ValueError, match="The exclude_same argument to InterRDF must be"): rdf = InterRDF(s2, s2, exclude_same="unsupported").run() + with pytest.raises(ValueError, match="The exclude_same argument to InterRDF cannot be used with"): + rdf = InterRDF(s2, s2, exclude_same="unsupported", exclude_block=tuple()).run() + + @pytest.mark.parametrize("attr", ("rdf", "bins", "edges", "count")) def test_rdf_attr_warning(sels, attr): s1, s2 = sels From df406de41bd13ebc946eb44800b8bdccfec5a1a7 Mon Sep 17 00:00:00 2001 From: Orion Cohen <27712051+orionarcher@users.noreply.github.com> Date: Thu, 13 Jul 2023 16:53:54 -0700 Subject: [PATCH 11/12] Update testsuite/MDAnalysisTests/analysis/test_rdf.py Co-authored-by: Rocco Meli --- testsuite/MDAnalysisTests/analysis/test_rdf.py | 1 - 1 file changed, 1 deletion(-) diff --git a/testsuite/MDAnalysisTests/analysis/test_rdf.py b/testsuite/MDAnalysisTests/analysis/test_rdf.py index e440d47aaa0..9ac48ef326f 100644 --- a/testsuite/MDAnalysisTests/analysis/test_rdf.py +++ b/testsuite/MDAnalysisTests/analysis/test_rdf.py @@ -109,7 +109,6 @@ def test_ignore_same_residues(sels, attr, count): def test_ignore_same_residues_fails(sels): - # should see two distances with 4 counts each s1, s2 = sels with pytest.raises(ValueError, match="The exclude_same argument to InterRDF must be"): rdf = InterRDF(s2, s2, exclude_same="unsupported").run() From 29606f2d1dc232c12192a891a4d5a99142830985 Mon Sep 17 00:00:00 2001 From: orionarcher Date: Tue, 18 Jul 2023 12:20:31 -0700 Subject: [PATCH 12/12] add test for exclusion_block + exclude_same --- testsuite/MDAnalysisTests/analysis/test_rdf.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testsuite/MDAnalysisTests/analysis/test_rdf.py b/testsuite/MDAnalysisTests/analysis/test_rdf.py index 9ac48ef326f..bc121666488 100644 --- a/testsuite/MDAnalysisTests/analysis/test_rdf.py +++ b/testsuite/MDAnalysisTests/analysis/test_rdf.py @@ -111,10 +111,10 @@ def test_ignore_same_residues(sels, attr, count): def test_ignore_same_residues_fails(sels): s1, s2 = sels with pytest.raises(ValueError, match="The exclude_same argument to InterRDF must be"): - rdf = InterRDF(s2, s2, exclude_same="unsupported").run() + InterRDF(s2, s2, exclude_same="unsupported").run() with pytest.raises(ValueError, match="The exclude_same argument to InterRDF cannot be used with"): - rdf = InterRDF(s2, s2, exclude_same="unsupported", exclude_block=tuple()).run() + InterRDF(s2, s2, exclude_same="residue", exclusion_block=tuple()).run() @pytest.mark.parametrize("attr", ("rdf", "bins", "edges", "count"))