Skip to content

Commit 15f4437

Browse files
authored
fix(dpmodel): mask virtual EnvMat centers (deepmodeling#5833)
## Summary - replace negative virtual-center types with a safe index before gathering normalization tables - use neutral average and standard-deviation values for masked centers, then explicitly zero their descriptors - add radial, angular, NumPy, and strict Array API regression coverage ## Why existing tests missed this The direct EnvMat unit test only used real center atoms. A separate model-level virtual-atom test did not expose the bug because the model masks virtual outputs after fitting and initializes descriptor averages to zero, so the invalid negative normalization lookup was not observable. The new tests use nonzero averages and a zero placeholder scale, which make both the negative-index behavior and masking order observable. The broader compiled DPA2 varying-natoms test had previously never been connected to virtual-center normalization; it is retained as a cross-cutting compile/autograd guard and was also reproduced after an isolated CI mismatch. ## Validation - ruff format . - ruff check . - focused dpmodel, Array API, PyTorch EnvMat, and virtual atomic-model tests: 7 passed with 2 subtests - TensorFlow C++ core regression test: 1 passed - `TestCompiledVaryingNatoms::test_compiled_matches_uncompiled_varying_natoms_dpa2`: passed locally in 205.82 s, covering four compiled/eager training steps, changing frame/atom counts, force and virial outputs, and second-order force-loss gradients - manual NumPy, Array API Strict, PyTorch, and JAX virtual-center scenario Closes deepmodeling#5628 Coding agent: Codex Codex version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning effort: xhigh <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved environment-matrix normalization for virtual center atoms. * Virtual centers now consistently produce zero environment, derivative, and switching outputs across supported computation modes. * **Tests** * Added coverage for all-virtual and mixed real/virtual center scenarios, including normalization with provided statistics. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: njzjz-bot <njzjz.bot@gmail.com>
1 parent 5902263 commit 15f4437

3 files changed

Lines changed: 116 additions & 3 deletions

File tree

deepmd/dpmodel/utils/env_mat.py

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,11 @@ def call(
172172
Parameters
173173
----------
174174
nlist
175-
The neighbor list. shape: nf x nloc x nnei
175+
The neighbor list. shape: nf x nloc x nnei. Entries equal to ``-1``
176+
mark empty neighbor slots, and a virtual center (whose atom type is
177+
negative) must have an entire neighbor row of ``-1``; the in-tree
178+
builder (``deepmd.dpmodel.utils.nlist.build_neighbor_list``) fills
179+
the full row of a virtual atom with ``-1`` by construction.
176180
coord_ext
177181
The extended coordinates of atoms. shape: nf x (nallx3)
178182
atype_ext
@@ -200,10 +204,34 @@ def call(
200204
em, diff, sw = self._call(nlist, coord_ext, radial_only)
201205
nf, nloc, nnei = nlist.shape
202206
atype = xp_take_first_n(atype_ext, 1, nloc)
207+
center_is_real = atype >= 0
208+
# Virtual atoms use a negative type sentinel. Never pass that sentinel to
209+
# ``take``: NumPy treats -1 as the final real type, while stricter array
210+
# namespaces may reject it. Type zero is only a safe placeholder because
211+
# the gathered rows are neutralized below.
212+
safe_atype = xp.where(center_is_real, atype, xp.zeros_like(atype))
213+
center_mask = xp.reshape(center_is_real, (nf, nloc, 1, 1))
214+
# ``_make_env_mat`` already zeroes em, diff and sw wherever ``nlist < 0``,
215+
# so a virtual center -- whose neighbor row is empty by the neighbor-list
216+
# contract -- leaves this function at zero as long as normalization does
217+
# not shift it. Neutralizing the offset and the scale is therefore the
218+
# whole fix; masking em/diff/sw again afterwards would make the
219+
# descriptor depend on ``atype_ext``, which the compiled pt_expt DPA2
220+
# lower miscompiles into wrong forces.
203221
if davg is not None:
204-
em -= xp.reshape(xp.take(davg, xp.reshape(atype, (-1,)), axis=0), em.shape)
222+
center_avg = xp.reshape(
223+
xp.take(davg, xp.reshape(safe_atype, (-1,)), axis=0), em.shape
224+
)
225+
center_avg = xp.where(center_mask, center_avg, xp.zeros_like(center_avg))
226+
em -= center_avg
205227
if dstd is not None:
206-
em /= xp.reshape(xp.take(dstd, xp.reshape(atype, (-1,)), axis=0), em.shape)
228+
center_std = xp.reshape(
229+
xp.take(dstd, xp.reshape(safe_atype, (-1,)), axis=0), em.shape
230+
)
231+
# A neutral scale avoids hidden divide-by-zero/NaN values in the
232+
# masked branch, which is important for differentiable backends.
233+
center_std = xp.where(center_mask, center_std, xp.ones_like(center_std))
234+
em /= center_std
207235
return em, diff, sw
208236

209237
def _call(

source/tests/common/dpmodel/array_api/test_env_mat.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import array_api_strict as xp
55

66
from deepmd.dpmodel.utils.env_mat import (
7+
EnvMat,
78
compute_smooth_weight,
89
)
910

@@ -23,3 +24,53 @@ def test_compute_smooth_weight(self) -> None:
2324
self.assert_namespace_equal(w, d)
2425
self.assert_device_equal(w, d)
2526
self.assert_dtype_equal(w, d)
27+
28+
def test_virtual_center_uses_safe_normalization_indices(self) -> None:
29+
"""Strict array indexing must never receive the negative type sentinel."""
30+
coord = xp.asarray([[0.0, 0.0, 0.0]], dtype=xp.float64)
31+
atype = xp.asarray([[-1]], dtype=xp.int64)
32+
nlist = xp.asarray([[[-1]]], dtype=xp.int64)
33+
davg = xp.asarray([[[11.0, 13.0, 17.0, 19.0]]], dtype=xp.float64)
34+
# A zero placeholder scale verifies that masking happens before division;
35+
# otherwise the virtual row can create hidden NaN or infinity values.
36+
dstd = xp.zeros_like(davg)
37+
38+
env_mat, diff, switch = EnvMat(2.0, 0.5).call(coord, atype, nlist, davg, dstd)
39+
40+
for output in (env_mat, diff, switch):
41+
self.assertTrue(bool(xp.all(output == xp.zeros_like(output))))
42+
self.assert_namespace_equal(output, coord)
43+
self.assert_device_equal(output, coord)
44+
self.assert_dtype_equal(output, coord)
45+
46+
def test_mixed_centers_keep_virtual_rows_at_zero(self) -> None:
47+
"""A real center is normalized; a virtual one is left untouched at zero."""
48+
coord = xp.asarray([[0.0, 0.0, 0.0, 1.0, 0.0, 0.0]], dtype=xp.float64)
49+
atype = xp.asarray([[0, -1]], dtype=xp.int64)
50+
# The virtual center's neighbor row is empty, per the neighbor-list
51+
# contract, so _make_env_mat leaves its outputs at zero. Only the
52+
# normalization below could shift them off zero.
53+
nlist = xp.asarray([[[1], [-1]]], dtype=xp.int64)
54+
# The last row is what an unguarded ``take`` selects for atype -1, so
55+
# keep it nonzero: borrowing it would shift the virtual row off zero.
56+
davg = xp.asarray(
57+
[[[0.0, 0.0, 0.0, 0.0]], [[11.0, 13.0, 17.0, 19.0]]],
58+
dtype=xp.float64,
59+
)
60+
dstd = xp.asarray(
61+
[[[1.0, 1.0, 1.0, 1.0]], [[2.0, 2.0, 2.0, 2.0]]],
62+
dtype=xp.float64,
63+
)
64+
65+
env_mat, diff, switch = EnvMat(2.0, 0.5).call(coord, atype, nlist, davg, dstd)
66+
67+
for output in (env_mat, diff, switch):
68+
real_output = output[:, :1, ...]
69+
virtual_output = output[:, 1:, ...]
70+
self.assertTrue(bool(xp.any(real_output != xp.zeros_like(real_output))))
71+
self.assertTrue(
72+
bool(xp.all(virtual_output == xp.zeros_like(virtual_output)))
73+
)
74+
self.assert_namespace_equal(output, coord)
75+
self.assert_device_equal(output, coord)
76+
self.assert_dtype_equal(output, coord)

source/tests/common/dpmodel/test_env_mat.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
)
1313
from .case_single_frame_with_nlist import (
1414
TestCaseSingleFrameWithNlist,
15+
TestCaseSingleFrameWithNlistWithVirtual,
1516
)
1617

1718

@@ -38,3 +39,36 @@ def test_self_consistency(
3839
np.testing.assert_allclose(mm0, mm1)
3940
np.testing.assert_allclose(diff0, diff1)
4041
np.testing.assert_allclose(ww0, ww1)
42+
43+
44+
class TestEnvMatWithVirtualCenter(
45+
unittest.TestCase, TestCaseSingleFrameWithNlistWithVirtual
46+
):
47+
def setUp(self) -> None:
48+
TestCaseSingleFrameWithNlistWithVirtual.setUp(self)
49+
50+
def test_normalization_keeps_virtual_centers_zero(self) -> None:
51+
"""Virtual centers must not borrow normalization data from a real type."""
52+
nf, nloc, nnei = self.nlist.shape
53+
virtual_center = self.atype_ext[:, :nloc] < 0
54+
55+
for radial_only, width in ((False, 4), (True, 1)):
56+
with self.subTest(radial_only=radial_only):
57+
# Nonzero values make accidental ``-1`` indexing observable: NumPy
58+
# would otherwise select the final real-type row silently.
59+
davg = np.arange(1, self.nt * nnei * width + 1, dtype=np.float64)
60+
davg = davg.reshape(self.nt, nnei, width)
61+
dstd = np.full_like(davg, 2.0)
62+
63+
env_mat, diff, switch = EnvMat(self.rcut, self.rcut_smth).call(
64+
self.coord_ext,
65+
self.atype_ext,
66+
self.nlist,
67+
davg,
68+
dstd,
69+
radial_only=radial_only,
70+
)
71+
72+
np.testing.assert_allclose(env_mat[virtual_center], 0.0)
73+
np.testing.assert_allclose(diff[virtual_center], 0.0)
74+
np.testing.assert_allclose(switch[virtual_center], 0.0)

0 commit comments

Comments
 (0)