Skip to content

Commit d8892c7

Browse files
wanghan-iapcmHan Wang
andauthored
fix(tf): reject non-prefix use_spin layouts in the spin helper (deepmodeling#5727)
## Problem Fixes deepmodeling#5680. The legacy TensorFlow spin implementation assumes spin-enabled types form a contiguous prefix of the type map. The SE-A `sel` extension takes the first `ntypes_spin` selections (`sel_a[:ntypes_spin]`), and the coordinate splitting (`deepmd/tf/descriptor/se_a.py`), force splitting (`deepmd/tf/model/ener.py`), and bias merging (`deepmd/tf/fit/ener.py`) all address the virtual block with a dense real-to-virtual offset (`i + len(use_spin)`). For a non-prefix layout such as `use_spin=[False, True]`, these read the wrong real/virtual type ranges or raise deep inside the graph, and nothing rejected the configuration up front. In practice all supported spin models list spin-enabled types first, so the broken layout went unnoticed. ## Approach The maintained PyTorch backend already handles the sparse/non-prefix layout via `Spin.spin_type`. Rather than refactor all four coupled sites in this legacy backend (which has almost no coverage and where a subtle mistake would silently corrupt training), this guards against the unsupported layout: the TF `Spin` helper now rejects a `use_spin` where a non-spin type precedes a spin-enabled one, with a message telling the user to list spin-enabled types first. This turns a silent-wrong result or an obscure crash into an actionable error and documents the invariant in one place. ## Test Adds `source/tests/tf/test_spin_prefix_guard.py`: non-prefix layouts (`[False, True]` and `[True, False, True]`) raise `ValueError`, while prefix layouts (`[True, False]`, `[True, True]`) and an all-non-spin list (`[False, False]`) are accepted. The existing TF spin model test uses a valid `[True, False]` prefix and continues to pass. The prefix requirement previously had no test. Co-authored-by: Han Wang <wang_han@iapcm.ac.cn>
1 parent aff66bd commit d8892c7

2 files changed

Lines changed: 63 additions & 0 deletions

File tree

deepmd/tf/utils/spin.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,25 @@ def __init__(
2626
virtual_len: list[float] | None = None,
2727
) -> None:
2828
"""Constructor."""
29+
# The TensorFlow spin implementation assumes spin-enabled types form a
30+
# contiguous prefix of the type map: the SE-A ``sel`` extension takes the
31+
# first ``ntypes_spin`` selections, and the coordinate/force splitting
32+
# and bias merging address the virtual block with a dense real->virtual
33+
# offset. Reject a layout where a non-spin type precedes a spin type,
34+
# which would silently read the wrong real/virtual type ranges.
35+
if use_spin is not None:
36+
seen_non_spin = False
37+
for flag in use_spin:
38+
if flag:
39+
if seen_non_spin:
40+
raise ValueError(
41+
"The TensorFlow spin implementation requires "
42+
"spin-enabled types (use_spin=True) to form a prefix "
43+
f"of the type map; got use_spin={use_spin}. List all "
44+
"spin-enabled types first in the type map."
45+
)
46+
else:
47+
seen_non_spin = True
2948
self.use_spin = use_spin
3049
self.spin_norm = spin_norm
3150
self.virtual_len = virtual_len
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# SPDX-License-Identifier: LGPL-3.0-or-later
2+
"""The TF Spin helper must reject non-prefix use_spin layouts.
3+
4+
The legacy TensorFlow spin implementation assumes spin-enabled types form a
5+
contiguous prefix of the type map: the SE-A ``sel`` extension takes the first
6+
``ntypes_spin`` selections, and the coordinate/force splitting and bias merging
7+
address the virtual block with a dense real->virtual offset. A non-prefix layout
8+
such as ``use_spin=[False, True]`` silently reads the wrong real/virtual type
9+
ranges (or raises deep inside the graph), so it must be rejected up front with a
10+
clear error.
11+
"""
12+
13+
import unittest
14+
15+
from deepmd.tf.utils.spin import (
16+
Spin,
17+
)
18+
19+
20+
class TestSpinPrefixGuard(unittest.TestCase):
21+
def test_non_prefix_rejected(self) -> None:
22+
with self.assertRaises(ValueError):
23+
Spin(use_spin=[False, True], spin_norm=[1.0], virtual_len=[0.4])
24+
25+
def test_non_prefix_rejected_middle(self) -> None:
26+
with self.assertRaises(ValueError):
27+
Spin(
28+
use_spin=[True, False, True],
29+
spin_norm=[1.0, 1.0],
30+
virtual_len=[0.4, 0.4],
31+
)
32+
33+
def test_prefix_accepted(self) -> None:
34+
# spin-enabled types first: the supported layout
35+
Spin(use_spin=[True, False], spin_norm=[1.0], virtual_len=[0.4])
36+
self.assertEqual(Spin(use_spin=[True, True]).ntypes_spin, 2)
37+
38+
def test_all_non_spin_accepted(self) -> None:
39+
# no spin types at all is not a non-prefix violation
40+
self.assertEqual(Spin(use_spin=[False, False]).ntypes_spin, 0)
41+
42+
43+
if __name__ == "__main__":
44+
unittest.main()

0 commit comments

Comments
 (0)