Skip to content

Commit 99104d7

Browse files
authored
Fix InterRDF_s normalization problem when given density = True (#121)
- Fixes #120 - Change the N=nA * nB to be 1, as this is a single-atom to single-atom RDF calculaton. - fix tests related to density - update CHANGELOG
1 parent e98ebcc commit 99104d7

File tree

3 files changed

+26
-13
lines changed

3 files changed

+26
-13
lines changed

CHANGELOG

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ Fixes
2828
- single-threaded --> synchronous
2929
- threaded --> threads
3030
* fixed RDF functions (gave wrong results if step != 1) (#114)
31+
* fixed InterRDF_s function (gave wrong results if density=True) (#120)
3132

3233
Changes
3334
* requires MDAnalysis >= 1.0.0 (#122)

pmda/rdf.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,9 +318,9 @@ def _conclude(self):
318318
# Empty lists to restore indices, RDF
319319
rdf = []
320320

321-
for i, (nA, nB) in enumerate(self.ag_shape):
321+
for i in range(len(self.ag_shape)):
322322
# Number of each selection
323-
N = nA * nB
323+
N = 1
324324

325325
# Average number density
326326
box_vol = self.volume / self.n_frames

pmda/test/test_rdf_s.py

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ def rdf_s(u, sels, scheduler):
4141
return InterRDF_s(u, sels).run()
4242

4343

44+
@pytest.fixture(scope='module')
45+
def rdf_ref(u):
46+
s1 = u.select_atoms('name ZND and resid 289')
47+
s2 = u.select_atoms(
48+
'name OD1 and resid 51 and sphzone 5.0 (resid 289)')
49+
return rdf.InterRDF(s1, s2).run()
50+
51+
4452
def test_nbins(u, sels):
4553
rdf = InterRDF_s(u, sels, nbins=412).run()
4654

@@ -100,13 +108,12 @@ def test_reduce(rdf_s):
100108

101109

102110
@pytest.mark.parametrize("n_blocks", [1, 2, 3, 4])
103-
def test_same_result(u, sels, n_blocks):
111+
def test_same_result(u, sels, n_blocks, rdf_ref):
104112
# should see same results from analysis.rdf.InterRDF_s
105113
# and pmda.rdf.InterRDF_s
106-
nrdf = rdf.InterRDF_s(u, sels).run()
107114
prdf = InterRDF_s(u, sels).run(n_blocks=n_blocks)
108-
assert_almost_equal(nrdf.count[0][0][0], prdf.count[0][0][0])
109-
assert_almost_equal(nrdf.rdf[0][0][0], prdf.rdf[0][0][0])
115+
assert_almost_equal(rdf_ref.count, prdf.count[0][0][0])
116+
assert_almost_equal(rdf_ref.rdf, prdf.rdf[0][0][0])
110117

111118

112119
@pytest.mark.parametrize("step", [1, 2, 3])
@@ -115,12 +122,17 @@ def test_trj_len(u, sels, step):
115122
nrdf = rdf.InterRDF_s(u, sels).run(step=step)
116123
prdf = InterRDF_s(u, sels).run(step=step)
117124
assert_almost_equal(nrdf.n_frames, prdf.n_frames)
118-
assert_almost_equal(nrdf.rdf[0][0][0], prdf.rdf[0][0][0])
119125

120126

121-
@pytest.mark.parametrize("density, value", [
122-
(True, 13275.775440503656),
123-
(False, 0.021915460340071267)])
124-
def test_density(u, sels, density, value):
125-
rdf = InterRDF_s(u, sels, density=density).run()
126-
assert_almost_equal(max(rdf.rdf[0][0][0]), value)
127+
@pytest.mark.parametrize("density", [True, False])
128+
def test_density(u, sels, density):
129+
s1 = u.select_atoms('name ZND and resid 289')
130+
s2 = u.select_atoms(
131+
'name OD1 and resid 51 and sphzone 5.0 (resid 289)')
132+
prdf = InterRDF_s(u, sels, density=density).run()
133+
if density:
134+
rdf_ref = rdf.InterRDF(s1, s2).run()
135+
assert_almost_equal(rdf_ref.rdf, prdf.rdf[0][0][0])
136+
else:
137+
nrdf = rdf.InterRDF_s(u, sels, density=density).run()
138+
assert_almost_equal(nrdf.rdf[0][0][0], prdf.rdf[0][0][0])

0 commit comments

Comments
 (0)