Skip to content

Commit a166808

Browse files
authored
fix(rdf): avoid mutating selection defaults (#1016)
1 parent db0cc99 commit a166808

2 files changed

Lines changed: 48 additions & 3 deletions

File tree

dpdata/md/rdf.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import numpy as np
44

55

6-
def rdf(sys, sel_type=[None, None], max_r=5, nbins=100):
6+
def rdf(sys, sel_type=None, max_r=5, nbins=100):
77
"""Compute the rdf of a system.
88
99
Parameters
@@ -40,7 +40,12 @@ def rdf(sys, sel_type=[None, None], max_r=5, nbins=100):
4040
)
4141

4242

43-
def compute_rdf(box, posis, atype, sel_type=[None, None], max_r=5, nbins=100):
43+
def compute_rdf(box, posis, atype, sel_type=None, max_r=5, nbins=100):
44+
"""Compute an RDF without mutating the caller's selection list."""
45+
if sel_type is None:
46+
sel_type = [None, None]
47+
else:
48+
sel_type = list(sel_type)
4449
nframes = box.shape[0]
4550
xx = None
4651
all_rdf = []
@@ -58,7 +63,13 @@ def compute_rdf(box, posis, atype, sel_type=[None, None], max_r=5, nbins=100):
5863
return xx, all_rdf, all_cod
5964

6065

61-
def _compute_rdf_1frame(box, posis, atype, sel_type=[None, None], max_r=5, nbins=100):
66+
def _compute_rdf_1frame(box, posis, atype, sel_type=None, max_r=5, nbins=100):
67+
if sel_type is None:
68+
sel_type = [None, None]
69+
else:
70+
# Normalise into a fresh list because the two ``None`` replacements
71+
# below must not leak into a caller-owned list or a function default.
72+
sel_type = list(sel_type)
6273
all_types = list(set(list(np.sort(atype, kind="stable"))))
6374
if sel_type[0] is None:
6475
sel_type[0] = all_types

tests/test_rdf.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
from __future__ import annotations
2+
3+
import unittest
4+
5+
import numpy as np
6+
7+
from dpdata.md.rdf import compute_rdf
8+
9+
try:
10+
import ase # noqa: F401
11+
except ModuleNotFoundError:
12+
skip_ase = True
13+
else:
14+
skip_ase = False
15+
16+
17+
@unittest.skipIf(skip_ase, "RDF calculation requires ASE")
18+
class TestRDFSelectionOwnership(unittest.TestCase):
19+
def test_default_selection_is_recomputed_per_system(self):
20+
box = np.eye(3).reshape(1, 3, 3) * 10
21+
positions = np.array([[[0.0, 0.0, 0.0], [1.0, 0.0, 0.0]]])
22+
rdf = compute_rdf
23+
24+
rdf(box, positions, np.array([0, 1]))
25+
self.assertEqual(rdf.__defaults__[0], None)
26+
_, values, _ = rdf(box, positions, np.array([2, 2]))
27+
self.assertTrue(np.isfinite(values).all())
28+
29+
def test_caller_selection_is_not_mutated(self):
30+
box = np.eye(3).reshape(1, 3, 3) * 10
31+
positions = np.array([[[0.0, 0.0, 0.0], [1.0, 0.0, 0.0]]])
32+
selection = [None, None]
33+
compute_rdf(box, positions, np.array([0, 1]), selection)
34+
self.assertEqual(selection, [None, None])

0 commit comments

Comments
 (0)