From 492e2e5a081bee2c47a932befef6bf965107154f Mon Sep 17 00:00:00 2001 From: hanaol Date: Sun, 26 Jul 2026 13:11:44 -0400 Subject: [PATCH 1/6] fix(pair-tab): validate uniform distance grid to avoid silently wrong potentials Signed-off-by: hanaol --- deepmd/utils/pair_tab.py | 9 ++++++ .../common/dpmodel/test_pairtab_preprocess.py | 30 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/deepmd/utils/pair_tab.py b/deepmd/utils/pair_tab.py index 503f721c98..9b135ba7e2 100644 --- a/deepmd/utils/pair_tab.py +++ b/deepmd/utils/pair_tab.py @@ -61,6 +61,15 @@ def reinit(self, filename: str, rcut: float | None = None) -> None: self.rmin = self.vdata[0][0] self.rmax = self.vdata[-1][0] self.hh = self.vdata[1][0] - self.vdata[0][0] + dx = np.diff(self.vdata[:, 0]) + if not np.allclose(dx, self.hh, rtol=1e-5, atol=1e-8): + raise ValueError( + f"The distance grid in the pairwise table {filename} is not " + "evenly spaced. The tabulated potential must be provided on a " + "uniform grid, but the stride inferred from the first two rows " + f"({self.hh}) does not match all distance intervals. Please " + "regrid the table to use a constant distance step." + ) ncol = self.vdata.shape[1] - 1 n0 = (-1 + np.sqrt(1 + 8 * ncol)) * 0.5 self.ntypes = int(n0 + 0.1) diff --git a/source/tests/common/dpmodel/test_pairtab_preprocess.py b/source/tests/common/dpmodel/test_pairtab_preprocess.py index 93a61bc1f6..089a18c426 100644 --- a/source/tests/common/dpmodel/test_pairtab_preprocess.py +++ b/source/tests/common/dpmodel/test_pairtab_preprocess.py @@ -275,5 +275,35 @@ def test_preprocess(self) -> None: ) +class TestPairTabGridSpacing(unittest.TestCase): + @patch("numpy.loadtxt") + def test_non_uniform_grid(self, mock_loadtxt) -> None: + mock_loadtxt.return_value = np.array( + [ + [0.00, 1.0], + [0.01, 0.8], + [0.02, 0.6], + [0.09, 0.3], + [0.16, 0.0], + ] + ) + with self.assertRaisesRegex(ValueError, "evenly spaced"): + PairTab(filename="dummy_path", rcut=0.16) + + @patch("numpy.loadtxt") + def test_uniform_grid(self, mock_loadtxt) -> None: + mock_loadtxt.return_value = np.array( + [ + [0.00, 1.0], + [0.01, 0.8], + [0.02, 0.6], + [0.03, 0.3], + [0.04, 0.0], + ] + ) + tab = PairTab(filename="dummy_path", rcut=0.04) + np.testing.assert_allclose(tab.hh, 0.01) + + if __name__ == "__main__": unittest.main(warnings="ignore") From f87eeb78ca040be26ca0ab307183c86ff0f4ff25 Mon Sep 17 00:00:00 2001 From: hanaol Date: Tue, 28 Jul 2026 15:26:37 -0400 Subject: [PATCH 2/6] fix(pair-tab): reject non-monotonic grids and keep reinit atomic Address review feedback on the pairwise table validation: - A constant zero or negative distance stride passed the uniform-spacing check, leaving hh == 0 (division by zero) or hh < 0 with rmin > rmax in the padding and extrapolation arithmetic. Require a strictly increasing grid before checking uniformity. - reinit() assigned vdata/rmin/rmax/hh before validating, so a failed reinit of a live PairTab left the new metadata next to the stale tab_info/tab_data. Validate locals first and commit instance state only once all checks pass. Add regression tests for duplicate and descending grids, and assert that a failed reinit leaves the serialized table unchanged. Co-Authored-By: Claude Opus 5 Signed-off-by: hanaol --- deepmd/utils/pair_tab.py | 37 +++++++---- .../common/dpmodel/test_pairtab_preprocess.py | 63 +++++++++++++++++++ 2 files changed, 89 insertions(+), 11 deletions(-) diff --git a/deepmd/utils/pair_tab.py b/deepmd/utils/pair_tab.py index 9b135ba7e2..0d3465dfff 100644 --- a/deepmd/utils/pair_tab.py +++ b/deepmd/utils/pair_tab.py @@ -57,26 +57,41 @@ def reinit(self, filename: str, rcut: float | None = None) -> None: if filename is None: self.tab_info, self.tab_data = None, None return - self.vdata = np.loadtxt(filename, dtype=self.data_type) - self.rmin = self.vdata[0][0] - self.rmax = self.vdata[-1][0] - self.hh = self.vdata[1][0] - self.vdata[0][0] - dx = np.diff(self.vdata[:, 0]) - if not np.allclose(dx, self.hh, rtol=1e-5, atol=1e-8): + # validate the table before committing any state to self, so that a + # failed reinit leaves an already-initialized object untouched. + vdata = np.loadtxt(filename, dtype=self.data_type) + rmin = vdata[0][0] + rmax = vdata[-1][0] + hh = vdata[1][0] - vdata[0][0] + dx = np.diff(vdata[:, 0]) + if not np.all(dx > 0): + raise ValueError( + f"The distance grid in the pairwise table {filename} is not " + "strictly increasing. The tabulated potential must be provided " + "on a uniform grid with distances sorted in ascending order and " + "without duplicated rows. Please regrid the table." + ) + if not np.allclose(dx, hh, rtol=1e-5, atol=1e-8): raise ValueError( f"The distance grid in the pairwise table {filename} is not " "evenly spaced. The tabulated potential must be provided on a " "uniform grid, but the stride inferred from the first two rows " - f"({self.hh}) does not match all distance intervals. Please " + f"({hh}) does not match all distance intervals. Please " "regrid the table to use a constant distance step." ) - ncol = self.vdata.shape[1] - 1 + ncol = vdata.shape[1] - 1 n0 = (-1 + np.sqrt(1 + 8 * ncol)) * 0.5 - self.ntypes = int(n0 + 0.1) - assert self.ntypes * (self.ntypes + 1) // 2 == ncol, ( - f"number of volumes provided in {filename} does not match guessed number of types {self.ntypes}" + ntypes = int(n0 + 0.1) + assert ntypes * (ntypes + 1) // 2 == ncol, ( + f"number of volumes provided in {filename} does not match guessed number of types {ntypes}" ) + self.vdata = vdata + self.rmin = rmin + self.rmax = rmax + self.hh = hh + self.ntypes = ntypes + # check table data against rcut and update tab_file if needed, table upper boundary is used as rcut if not provided. self.rcut = rcut if rcut is not None else self.rmax self._check_table_upper_boundary() diff --git a/source/tests/common/dpmodel/test_pairtab_preprocess.py b/source/tests/common/dpmodel/test_pairtab_preprocess.py index 089a18c426..632cbc0c0f 100644 --- a/source/tests/common/dpmodel/test_pairtab_preprocess.py +++ b/source/tests/common/dpmodel/test_pairtab_preprocess.py @@ -290,6 +290,69 @@ def test_non_uniform_grid(self, mock_loadtxt) -> None: with self.assertRaisesRegex(ValueError, "evenly spaced"): PairTab(filename="dummy_path", rcut=0.16) + @patch("numpy.loadtxt") + def test_duplicate_distances(self, mock_loadtxt) -> None: + # a constant zero stride passes the uniformity check but yields hh == 0 + mock_loadtxt.return_value = np.array( + [ + [0.01, 1.0], + [0.01, 0.8], + [0.01, 0.6], + [0.01, 0.0], + ] + ) + with self.assertRaisesRegex(ValueError, "strictly increasing"): + PairTab(filename="dummy_path", rcut=0.04) + + @patch("numpy.loadtxt") + def test_descending_grid(self, mock_loadtxt) -> None: + # a constant negative stride passes the uniformity check but yields hh < 0 + mock_loadtxt.return_value = np.array( + [ + [0.04, 1.0], + [0.03, 0.8], + [0.02, 0.6], + [0.01, 0.0], + ] + ) + with self.assertRaisesRegex(ValueError, "strictly increasing"): + PairTab(filename="dummy_path", rcut=0.04) + + @patch("numpy.loadtxt") + def test_failed_reinit_keeps_state(self, mock_loadtxt) -> None: + uniform = np.array( + [ + [0.00, 1.0], + [0.01, 0.8], + [0.02, 0.6], + [0.03, 0.3], + [0.04, 0.0], + ] + ) + mock_loadtxt.return_value = uniform + tab = PairTab(filename="dummy_path", rcut=0.04) + expected = tab.serialize() + + mock_loadtxt.return_value = np.array( + [ + [0.00, 1.0], + [0.01, 0.8], + [0.02, 0.6], + [0.09, 0.3], + [0.16, 0.0], + ] + ) + with self.assertRaisesRegex(ValueError, "evenly spaced"): + tab.reinit(filename="dummy_path", rcut=0.16) + + actual = tab.serialize() + for key in ("rmin", "rmax", "hh", "ntypes", "rcut", "nspline"): + self.assertEqual(actual[key], expected[key]) + for key in ("vdata", "tab_info", "tab_data"): + np.testing.assert_allclose( + actual["@variables"][key], expected["@variables"][key] + ) + @patch("numpy.loadtxt") def test_uniform_grid(self, mock_loadtxt) -> None: mock_loadtxt.return_value = np.array( From 3660ba20a50475f634d578d1a9776ae3c24471ff Mon Sep 17 00:00:00 2001 From: hanaol Date: Tue, 28 Jul 2026 15:40:26 -0400 Subject: [PATCH 3/6] test(pair-tab): snapshot serialized state before failed reinit serialize() returns references to the live vdata/tab_info/tab_data arrays, so deep-copy the expected snapshot rather than aliasing it, and compare the arrays exactly since a failed reinit must leave them untouched. Co-Authored-By: Claude Opus 5 Signed-off-by: hanaol --- source/tests/common/dpmodel/test_pairtab_preprocess.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/source/tests/common/dpmodel/test_pairtab_preprocess.py b/source/tests/common/dpmodel/test_pairtab_preprocess.py index 632cbc0c0f..76e5d23ee5 100644 --- a/source/tests/common/dpmodel/test_pairtab_preprocess.py +++ b/source/tests/common/dpmodel/test_pairtab_preprocess.py @@ -1,4 +1,5 @@ # SPDX-License-Identifier: LGPL-3.0-or-later +import copy import unittest from unittest.mock import ( patch, @@ -331,7 +332,8 @@ def test_failed_reinit_keeps_state(self, mock_loadtxt) -> None: ) mock_loadtxt.return_value = uniform tab = PairTab(filename="dummy_path", rcut=0.04) - expected = tab.serialize() + # serialize() hands back the live arrays, so snapshot them + expected = copy.deepcopy(tab.serialize()) mock_loadtxt.return_value = np.array( [ @@ -349,7 +351,7 @@ def test_failed_reinit_keeps_state(self, mock_loadtxt) -> None: for key in ("rmin", "rmax", "hh", "ntypes", "rcut", "nspline"): self.assertEqual(actual[key], expected[key]) for key in ("vdata", "tab_info", "tab_data"): - np.testing.assert_allclose( + np.testing.assert_array_equal( actual["@variables"][key], expected["@variables"][key] ) From 772e1c285c0a00cb849546fc42b891425f8f3e3c Mon Sep 17 00:00:00 2001 From: hanaol Date: Tue, 28 Jul 2026 15:45:37 -0400 Subject: [PATCH 4/6] style(pair-tab): drop explanatory comments from validation and tests Co-Authored-By: Claude Opus 5 Signed-off-by: hanaol --- deepmd/utils/pair_tab.py | 2 -- source/tests/common/dpmodel/test_pairtab_preprocess.py | 3 --- 2 files changed, 5 deletions(-) diff --git a/deepmd/utils/pair_tab.py b/deepmd/utils/pair_tab.py index 0d3465dfff..81cd4326b4 100644 --- a/deepmd/utils/pair_tab.py +++ b/deepmd/utils/pair_tab.py @@ -57,8 +57,6 @@ def reinit(self, filename: str, rcut: float | None = None) -> None: if filename is None: self.tab_info, self.tab_data = None, None return - # validate the table before committing any state to self, so that a - # failed reinit leaves an already-initialized object untouched. vdata = np.loadtxt(filename, dtype=self.data_type) rmin = vdata[0][0] rmax = vdata[-1][0] diff --git a/source/tests/common/dpmodel/test_pairtab_preprocess.py b/source/tests/common/dpmodel/test_pairtab_preprocess.py index 76e5d23ee5..3143442cbb 100644 --- a/source/tests/common/dpmodel/test_pairtab_preprocess.py +++ b/source/tests/common/dpmodel/test_pairtab_preprocess.py @@ -293,7 +293,6 @@ def test_non_uniform_grid(self, mock_loadtxt) -> None: @patch("numpy.loadtxt") def test_duplicate_distances(self, mock_loadtxt) -> None: - # a constant zero stride passes the uniformity check but yields hh == 0 mock_loadtxt.return_value = np.array( [ [0.01, 1.0], @@ -307,7 +306,6 @@ def test_duplicate_distances(self, mock_loadtxt) -> None: @patch("numpy.loadtxt") def test_descending_grid(self, mock_loadtxt) -> None: - # a constant negative stride passes the uniformity check but yields hh < 0 mock_loadtxt.return_value = np.array( [ [0.04, 1.0], @@ -332,7 +330,6 @@ def test_failed_reinit_keeps_state(self, mock_loadtxt) -> None: ) mock_loadtxt.return_value = uniform tab = PairTab(filename="dummy_path", rcut=0.04) - # serialize() hands back the live arrays, so snapshot them expected = copy.deepcopy(tab.serialize()) mock_loadtxt.return_value = np.array( From 040569edc1a5c8b80a83480d9d277c34eeedb249 Mon Sep 17 00:00:00 2001 From: hanaol Date: Wed, 29 Jul 2026 11:07:54 -0400 Subject: [PATCH 5/6] fix(pair-tab): use a scale-aware tolerance for the grid spacing check atol=1e-8 dominated the comparison for sub-nanometre grids, so intervals differing by an order of magnitude still compared equal and the table was encoded with the smaller stride. Drop the absolute term and rely on rtol, which is scale-invariant. Co-Authored-By: Claude Opus 5 Signed-off-by: hanaol --- deepmd/utils/pair_tab.py | 2 +- .../common/dpmodel/test_pairtab_preprocess.py | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/deepmd/utils/pair_tab.py b/deepmd/utils/pair_tab.py index 81cd4326b4..24711b97d4 100644 --- a/deepmd/utils/pair_tab.py +++ b/deepmd/utils/pair_tab.py @@ -69,7 +69,7 @@ def reinit(self, filename: str, rcut: float | None = None) -> None: "on a uniform grid with distances sorted in ascending order and " "without duplicated rows. Please regrid the table." ) - if not np.allclose(dx, hh, rtol=1e-5, atol=1e-8): + if not np.allclose(dx, hh, rtol=1e-5, atol=0): raise ValueError( f"The distance grid in the pairwise table {filename} is not " "evenly spaced. The tabulated potential must be provided on a " diff --git a/source/tests/common/dpmodel/test_pairtab_preprocess.py b/source/tests/common/dpmodel/test_pairtab_preprocess.py index 3143442cbb..a240c10d7c 100644 --- a/source/tests/common/dpmodel/test_pairtab_preprocess.py +++ b/source/tests/common/dpmodel/test_pairtab_preprocess.py @@ -317,6 +317,34 @@ def test_descending_grid(self, mock_loadtxt) -> None: with self.assertRaisesRegex(ValueError, "strictly increasing"): PairTab(filename="dummy_path", rcut=0.04) + @patch("numpy.loadtxt") + def test_non_uniform_fine_grid(self, mock_loadtxt) -> None: + mock_loadtxt.return_value = np.array( + [ + [0.0, 1.0], + [1e-10, 0.8], + [1.1e-9, 0.6], + [2.1e-9, 0.3], + [3.1e-9, 0.0], + ] + ) + with self.assertRaisesRegex(ValueError, "evenly spaced"): + PairTab(filename="dummy_path", rcut=3.1e-9) + + @patch("numpy.loadtxt") + def test_uniform_fine_grid(self, mock_loadtxt) -> None: + mock_loadtxt.return_value = np.array( + [ + [0.0, 1.0], + [1e-9, 0.8], + [2e-9, 0.6], + [3e-9, 0.3], + [4e-9, 0.0], + ] + ) + tab = PairTab(filename="dummy_path", rcut=4e-9) + self.assertAlmostEqual(tab.hh, 1e-9) + @patch("numpy.loadtxt") def test_failed_reinit_keeps_state(self, mock_loadtxt) -> None: uniform = np.array( From 2aa6704001c0f6f71243ffe63fc69ebb604d0bd6 Mon Sep 17 00:00:00 2001 From: hanaol Date: Sun, 2 Aug 2026 12:08:00 -0400 Subject: [PATCH 6/6] fix(pair-tab): validate grid by absolute node position, not interval --- deepmd/utils/pair_tab.py | 17 +++++++++++++---- .../common/dpmodel/test_pairtab_preprocess.py | 11 +++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/deepmd/utils/pair_tab.py b/deepmd/utils/pair_tab.py index 24711b97d4..e951cc7f30 100644 --- a/deepmd/utils/pair_tab.py +++ b/deepmd/utils/pair_tab.py @@ -69,13 +69,22 @@ def reinit(self, filename: str, rcut: float | None = None) -> None: "on a uniform grid with distances sorted in ascending order and " "without duplicated rows. Please regrid the table." ) - if not np.allclose(dx, hh, rtol=1e-5, atol=0): + # validate against absolute node positions rather than per-interval + # spacing: consumers (the C++ kernel and _make_data) index by + # rmin + i * hh, so that is what must stay accurate, not each dx. + n = vdata.shape[0] + hh_ref = (rmax - rmin) / (n - 1) + deviation = np.abs(vdata[:, 0] - (rmin + hh_ref * np.arange(n))) + tol = 1e-2 * abs(hh_ref) + if np.any(deviation > tol): + bad_row = int(np.argmax(deviation > tol)) raise ValueError( f"The distance grid in the pairwise table {filename} is not " "evenly spaced. The tabulated potential must be provided on a " - "uniform grid, but the stride inferred from the first two rows " - f"({hh}) does not match all distance intervals. Please " - "regrid the table to use a constant distance step." + f"uniform grid, but row {bad_row} (distance " + f"{vdata[bad_row, 0]}) does not match the constant step " + f"inferred from rmin and rmax ({hh_ref}). Please regrid the " + "table to use a constant distance step." ) ncol = vdata.shape[1] - 1 n0 = (-1 + np.sqrt(1 + 8 * ncol)) * 0.5 diff --git a/source/tests/common/dpmodel/test_pairtab_preprocess.py b/source/tests/common/dpmodel/test_pairtab_preprocess.py index a240c10d7c..e194a624cc 100644 --- a/source/tests/common/dpmodel/test_pairtab_preprocess.py +++ b/source/tests/common/dpmodel/test_pairtab_preprocess.py @@ -1,5 +1,7 @@ # SPDX-License-Identifier: LGPL-3.0-or-later import copy +import os +import tempfile import unittest from unittest.mock import ( patch, @@ -394,6 +396,15 @@ def test_uniform_grid(self, mock_loadtxt) -> None: tab = PairTab(filename="dummy_path", rcut=0.04) np.testing.assert_allclose(tab.hh, 0.01) + def test_uniform_grid_rounded_text_precision(self) -> None: + rr = np.linspace(0.0, 6.0, 1000) + ee = np.exp(-rr) + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "table.txt") + np.savetxt(path, np.stack((rr, ee), axis=1), fmt="%.6f") + tab = PairTab(filename=path) + self.assertAlmostEqual(tab.hh, rr[1] - rr[0], places=6) + if __name__ == "__main__": unittest.main(warnings="ignore")