From bfc267806359abb60e03dfd4601a44dd4184cfcc Mon Sep 17 00:00:00 2001 From: njzjz-bot Date: Thu, 16 Jul 2026 12:24:08 +0800 Subject: [PATCH 1/2] fix(gromacs): preserve inferred atom type order Infer GROMACS atom types in first-seen order when no type_map is supplied. Add a regression fixture matching issue #740 so both atom names and type IDs are checked instead of membership alone. Coding-Agent: Codex Codex-Version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning-Effort: xhigh --- dpdata/formats/gromacs/gro.py | 7 ++++++- tests/gromacs/type_order.gro | 7 +++++++ tests/test_gromacs_gro.py | 10 ++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 tests/gromacs/type_order.gro diff --git a/dpdata/formats/gromacs/gro.py b/dpdata/formats/gromacs/gro.py index 0c61544fd..a5950c7be 100644 --- a/dpdata/formats/gromacs/gro.py +++ b/dpdata/formats/gromacs/gro.py @@ -77,7 +77,12 @@ def file_to_system_data(fname: FileType, format_atom_name=True, **kwargs): posis = np.array(posis) if frame == 1: system["orig"] = np.zeros(3) - system["atom_names"] = list(set(names)) + # A .gro file does not carry a separate atom-type table. When + # callers do not provide ``type_map``, infer that table from + # the first occurrence of each atom name. ``dict`` preserves + # insertion order, unlike ``set`` whose hash-dependent order + # could silently change the type IDs used by later exporters. + system["atom_names"] = list(dict.fromkeys(names)) system["atom_numbs"] = [ names.count(ii) for ii in system["atom_names"] ] diff --git a/tests/gromacs/type_order.gro b/tests/gromacs/type_order.gro new file mode 100644 index 000000000..ac52c7386 --- /dev/null +++ b/tests/gromacs/type_order.gro @@ -0,0 +1,7 @@ +Generated with MDTraj, t= 0.0 + 4 + 1MOL LI 1 0.254 0.254 0.024 + 2MOL CL 2 0.507 0.000 0.000 + 3MOL P 3 0.507 0.507 0.507 + 4MOL S1 4 0.896 0.389 0.118 + 1.01400 1.01400 1.01400 0.00000 0.00000 0.00000 0.00000 0.00000 0.00000 diff --git a/tests/test_gromacs_gro.py b/tests/test_gromacs_gro.py index 674c65100..a779a8f4b 100644 --- a/tests/test_gromacs_gro.py +++ b/tests/test_gromacs_gro.py @@ -7,6 +7,16 @@ class TestGromacsGro(unittest.TestCase): + def test_infer_atom_types_in_first_seen_order(self): + # Without an explicit type_map, atom names define the type IDs in the + # order in which they first occur in the .gro file. Checking membership + # alone would miss the order regression reported for this conversion. + system = dpdata.System("gromacs/type_order.gro") + + self.assertEqual(system["atom_names"], ["Li", "Cl", "P", "S"]) + self.assertEqual(system["atom_numbs"], [1, 1, 1, 1]) + self.assertEqual(system["atom_types"].tolist(), [0, 1, 2, 3]) + def test_read_file(self): system = dpdata.System("gromacs/1h.gro", type_map=["H", "O"]) self.assertTrue("H" in system["atom_names"]) From d2e89562134720a1acbf998a2f63ec00ea22ec94 Mon Sep 17 00:00:00 2001 From: njzjz-bot Date: Sun, 19 Jul 2026 22:46:09 +0800 Subject: [PATCH 2/2] test(gromacs): strengthen atom order regression Coding-Agent: Codex Codex-Version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning-Effort: xhigh --- dpdata/formats/gromacs/gro.py | 10 +++++----- tests/gromacs/type_order.gro | 10 +++++++++- tests/test_gromacs_gro.py | 28 ++++++++++++++++++++++------ 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/dpdata/formats/gromacs/gro.py b/dpdata/formats/gromacs/gro.py index a5950c7be..413a1ff23 100644 --- a/dpdata/formats/gromacs/gro.py +++ b/dpdata/formats/gromacs/gro.py @@ -77,11 +77,11 @@ def file_to_system_data(fname: FileType, format_atom_name=True, **kwargs): posis = np.array(posis) if frame == 1: system["orig"] = np.zeros(3) - # A .gro file does not carry a separate atom-type table. When - # callers do not provide ``type_map``, infer that table from - # the first occurrence of each atom name. ``dict`` preserves - # insertion order, unlike ``set`` whose hash-dependent order - # could silently change the type IDs used by later exporters. + # A .gro file does not carry a separate atom-type table. The + # original reader sorted inferred names alphabetically, but + # the multi-frame refactor accidentally retained an unsorted + # set. Use first occurrence as the explicit contract so type + # IDs remain stable and follow the file's atom order. system["atom_names"] = list(dict.fromkeys(names)) system["atom_numbs"] = [ names.count(ii) for ii in system["atom_names"] diff --git a/tests/gromacs/type_order.gro b/tests/gromacs/type_order.gro index ac52c7386..37b0a331f 100644 --- a/tests/gromacs/type_order.gro +++ b/tests/gromacs/type_order.gro @@ -1,7 +1,15 @@ Generated with MDTraj, t= 0.0 - 4 + 12 1MOL LI 1 0.254 0.254 0.024 2MOL CL 2 0.507 0.000 0.000 3MOL P 3 0.507 0.507 0.507 4MOL S1 4 0.896 0.389 0.118 + 5MOL O 5 0.100 0.200 0.300 + 6MOL N 6 0.200 0.300 0.400 + 7MOL C 7 0.300 0.400 0.500 + 8MOL H 8 0.400 0.500 0.600 + 9MOL F 9 0.500 0.600 0.700 + 10MOL BR 10 0.600 0.700 0.800 + 11MOL I 11 0.700 0.800 0.900 + 12MOL NA 12 0.800 0.900 1.000 1.01400 1.01400 1.01400 0.00000 0.00000 0.00000 0.00000 0.00000 0.00000 diff --git a/tests/test_gromacs_gro.py b/tests/test_gromacs_gro.py index a779a8f4b..2b554210f 100644 --- a/tests/test_gromacs_gro.py +++ b/tests/test_gromacs_gro.py @@ -8,14 +8,30 @@ class TestGromacsGro(unittest.TestCase): def test_infer_atom_types_in_first_seen_order(self): - # Without an explicit type_map, atom names define the type IDs in the - # order in which they first occur in the .gro file. Checking membership - # alone would miss the order regression reported for this conversion. + # Without an explicit type_map, atom names define type IDs in first-seen + # order. The fixture uses many distinct names so the former set-based + # implementation cannot routinely pass by coincidental hash ordering. system = dpdata.System("gromacs/type_order.gro") - self.assertEqual(system["atom_names"], ["Li", "Cl", "P", "S"]) - self.assertEqual(system["atom_numbs"], [1, 1, 1, 1]) - self.assertEqual(system["atom_types"].tolist(), [0, 1, 2, 3]) + expected_names = [ + "Li", + "Cl", + "P", + "S", + "O", + "N", + "C", + "H", + "F", + "Br", + "I", + "Na", + ] + self.assertEqual(system["atom_names"], expected_names) + self.assertEqual(system["atom_numbs"], [1] * len(expected_names)) + self.assertEqual( + system["atom_types"].tolist(), list(range(len(expected_names))) + ) def test_read_file(self): system = dpdata.System("gromacs/1h.gro", type_map=["H", "O"])