Skip to content

Commit ecad7fa

Browse files
committed
fix(charge-state): enforce the shared table domain at every host boundary
DPA3, DPA4 and DPA4C all embed the frame condition by casting it to int64 and gathering one row of a 200-entry charge table and one of a 100-entry multiplicity table. The domain is therefore a property of the feature, not of any descriptor, so the per-descriptor declaration chain added earlier is replaced by one shared contract in `deepmd.utils.charge_state`. All three descriptors size their tables from it and validate their configured default against it, which closes the silent truncation that let DPA3 accept a fractional state. The archive keeps carrying the ranges so the C++ boundary enforces the same rule without a Python model and without recompiling when the domain changes. `check_call_charge_spin` and `set_charge_spin` now reject a state that addresses no row on the per-call, folded and default paths; an archive frozen before the ranges were recorded is checked on width alone, as before.
1 parent 800bab1 commit ecad7fa

25 files changed

Lines changed: 334 additions & 307 deletions

deepmd/dpmodel/atomic_model/base_atomic_model.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,10 +254,6 @@ def get_default_chg_spin(self) -> list[float] | None:
254254
"""Get the default charge_spin values."""
255255
return None
256256

257-
def get_chg_spin_table_ranges(self) -> list[tuple[int, int]] | None:
258-
"""Get the row range each charge_spin value indexes, or None."""
259-
return None
260-
261257
def reinit_atom_exclude(
262258
self,
263259
exclude_types: list[int] = [],

deepmd/dpmodel/atomic_model/dp_atomic_model.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,6 @@ def get_default_chg_spin(self) -> list[float] | None:
154154
return self.descriptor.get_default_chg_spin()
155155
return None
156156

157-
def get_chg_spin_table_ranges(self) -> list[tuple[int, int]] | None:
158-
"""Get the row range each charge_spin value indexes, or None."""
159-
if self.add_chg_spin_ebd:
160-
return self.descriptor.get_chg_spin_table_ranges()
161-
return None
162-
163157
def uses_graph_lower(self) -> bool:
164158
"""Delegates to this model's own descriptor."""
165159
return bool(self.descriptor.uses_graph_lower())

deepmd/dpmodel/atomic_model/linear_atomic_model.py

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -740,20 +740,6 @@ def get_default_chg_spin(self) -> "Array | None":
740740
lambda m: m.get_default_chg_spin(),
741741
)[1]
742742

743-
def get_chg_spin_table_ranges(self) -> list[tuple[int, int]] | None:
744-
"""The shared table row ranges, if the children agree.
745-
746-
A condition reaches every consuming child, so it must address the
747-
tables of all of them. Children that disagree share no acceptable
748-
state, which the composition reports as an unconstrained domain
749-
rather than silently enforcing one child's tables on the others.
750-
"""
751-
return self._agreed_default(
752-
self._chg_spin_consumers(),
753-
lambda m: m.get_chg_spin_table_ranges() is not None,
754-
lambda m: m.get_chg_spin_table_ranges(),
755-
)[1]
756-
757743
def has_default_fparam(self) -> bool:
758744
"""Whether every active child shares one default frame parameter."""
759745
return self._agreed_default(

deepmd/dpmodel/descriptor/dpa3.py

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@
3333
from deepmd.dpmodel.utils.update_sel import (
3434
UpdateSel,
3535
)
36+
from deepmd.utils.charge_state import (
37+
CHARGE_OFFSET,
38+
CHARGE_TABLE_ROWS,
39+
MULTIPLICITY_TABLE_ROWS,
40+
validate_charge_state,
41+
)
3642
from deepmd.utils.data_system import (
3743
DeepmdDataSystem,
3844
)
@@ -468,11 +474,11 @@ def init_subclass_params(sub_data: dict | Any, sub_class: type) -> Any:
468474

469475
self.use_econf_tebd = use_econf_tebd
470476
self.add_chg_spin_ebd = add_chg_spin_ebd
471-
if default_chg_spin is not None and len(default_chg_spin) != 2:
472-
raise ValueError(
473-
"default_chg_spin must have exactly 2 values [charge, spin]"
474-
)
475-
self.default_chg_spin = default_chg_spin
477+
self.default_chg_spin = (
478+
None
479+
if default_chg_spin is None
480+
else validate_charge_state(default_chg_spin)
481+
)
476482
self.use_tebd_bias = use_tebd_bias
477483
self.use_loc_mapping = use_loc_mapping
478484
self.type_map = type_map
@@ -494,18 +500,16 @@ def init_subclass_params(sub_data: dict | Any, sub_class: type) -> Any:
494500

495501
if self.add_chg_spin_ebd:
496502
self.cs_activation_fn = get_activation_fn(activation_function)
497-
# -100 ~ 100 is a conservative bound
498503
self.chg_embedding = TypeEmbedNet(
499-
ntypes=200,
504+
ntypes=CHARGE_TABLE_ROWS,
500505
neuron=[self.tebd_dim],
501506
padding=True,
502507
activation_function="Linear",
503508
precision=precision,
504509
seed=child_seed(seed, 3),
505510
)
506-
# 100 is a conservative upper bound
507511
self.spin_embedding = TypeEmbedNet(
508-
ntypes=100,
512+
ntypes=MULTIPLICITY_TABLE_ROWS,
509513
neuron=[self.tebd_dim],
510514
padding=True,
511515
activation_function="Linear",
@@ -755,7 +759,7 @@ def call(
755759
assert self.spin_embedding is not None
756760
chg_tebd = self.chg_embedding.call()
757761
spin_tebd = self.spin_embedding.call()
758-
charge = xp.astype(charge_spin[:, 0], xp.int64) + 100
762+
charge = xp.astype(charge_spin[:, 0], xp.int64) + CHARGE_OFFSET
759763
spin = xp.astype(charge_spin[:, 1], xp.int64)
760764
chg_ebd = xp.reshape(
761765
xp.take(chg_tebd, xp.reshape(charge, (-1,)), axis=0),

deepmd/dpmodel/descriptor/dpa4.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@
7373
from deepmd.dpmodel.utils.update_sel import (
7474
UpdateSel,
7575
)
76+
from deepmd.utils.charge_state import (
77+
validate_charge_state,
78+
)
7679
from deepmd.utils.version import (
7780
check_version_compatibility,
7881
)
@@ -784,10 +787,10 @@ def __init__(
784787
self.edge_cartesian = bool(edge_cartesian)
785788
self.node_cartesian = str(node_cartesian)
786789
self.add_chg_spin_ebd = bool(add_chg_spin_ebd)
787-
if default_chg_spin is not None and len(default_chg_spin) != 2:
788-
raise ValueError("`default_chg_spin` must contain [charge, spin].")
789790
self.default_chg_spin = (
790-
None if default_chg_spin is None else [float(x) for x in default_chg_spin]
791+
None
792+
if default_chg_spin is None
793+
else validate_charge_state(default_chg_spin)
791794
)
792795

793796
# === Native per-atom spin embedding ===

deepmd/dpmodel/descriptor/dpa4_nn/embedding.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@
4545
from deepmd.dpmodel.utils.type_embed import (
4646
remap_atype_to_padding,
4747
)
48+
from deepmd.utils.charge_state import (
49+
CHARGE_OFFSET,
50+
CHARGE_TABLE_ROWS,
51+
MULTIPLICITY_TABLE_ROWS,
52+
)
4853
from deepmd.utils.version import (
4954
check_version_compatibility,
5055
)
@@ -884,15 +889,15 @@ def __init__(
884889
raise ValueError("`embed_dim` must be positive")
885890

886891
self.charge_embedding = SeZMTypeEmbedding(
887-
ntypes=200,
892+
ntypes=CHARGE_TABLE_ROWS,
888893
embed_dim=self.embed_dim,
889894
precision=self.precision,
890895
seed=child_seed(seed, 0),
891896
trainable=self.trainable,
892897
padding=False,
893898
)
894899
self.spin_embedding = SeZMTypeEmbedding(
895-
ntypes=100,
900+
ntypes=MULTIPLICITY_TABLE_ROWS,
896901
embed_dim=self.embed_dim,
897902
precision=self.precision,
898903
seed=child_seed(seed, 1),
@@ -923,7 +928,7 @@ def call(self, charge_spin: Any) -> Any:
923928
Mixed condition embedding with shape (nf, embed_dim).
924929
"""
925930
xp = array_api_compat.array_namespace(charge_spin)
926-
charge = xp.astype(charge_spin[:, 0], xp.int64) + 100
931+
charge = xp.astype(charge_spin[:, 0], xp.int64) + CHARGE_OFFSET
927932
spin = xp.astype(charge_spin[:, 1], xp.int64)
928933
charge_embed = self.charge_embedding(charge)
929934
spin_embed = self.spin_embedding(spin)

deepmd/dpmodel/descriptor/dpa4c.py

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@
5858
from deepmd.dpmodel.utils.update_sel import (
5959
UpdateSel,
6060
)
61+
from deepmd.utils.charge_state import (
62+
validate_charge_state,
63+
)
6164
from deepmd.utils.version import (
6265
check_version_compatibility,
6366
)
@@ -73,7 +76,6 @@
7376
resolve_swiglu_hidden_width,
7477
)
7578
from .dpa4c_nn import (
76-
CHARGE_STATE_TABLE_RANGES,
7779
ChargeStateEmbedding,
7880
InvariantReadout,
7981
OrderedPairFiLM,
@@ -85,7 +87,6 @@
8587
derive_bispectrum_ranks,
8688
derive_degree_channels,
8789
derive_spin_channels,
88-
validate_charge_state,
8990
)
9091

9192
if TYPE_CHECKING:
@@ -2004,19 +2005,6 @@ def get_default_chg_spin(self) -> list[float] | None:
20042005
"""Return the fallback ``[charge, multiplicity]``, if configured."""
20052006
return self.default_chg_spin
20062007

2007-
def get_chg_spin_table_ranges(self) -> list[tuple[int, int]] | None:
2008-
"""Return the row range each value of the frame condition indexes.
2009-
2010-
The condition is embedded by gathering one row of the charge table and
2011-
one of the multiplicity table, so an acceptable state is a pair of
2012-
integers inside these half-open ranges. A folded condition indexes the
2013-
same tables at rebuild time, so the ranges hold whether or not the
2014-
descriptor is compressed.
2015-
"""
2016-
if self.charge_spin_embedding is None:
2017-
return None
2018-
return [tuple(rng) for rng in CHARGE_STATE_TABLE_RANGES]
2019-
20202008
def has_message_passing_across_ranks(self) -> bool:
20212009
"""Return whether intermediate halo communication is required."""
20222010
return False

deepmd/dpmodel/descriptor/dpa4c_nn/__init__.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,8 @@
88
enumerate_degree_triples,
99
)
1010
from .charge_state import (
11-
CHARGE_STATE_TABLE_RANGES,
1211
ChargeStateEmbedding,
1312
canonicalize_charge_spin,
14-
validate_charge_state,
1513
)
1614
from .geometry import (
1715
MAX_ANGULAR_DEGREE,
@@ -34,7 +32,6 @@
3432
)
3533

3634
__all__ = [
37-
"CHARGE_STATE_TABLE_RANGES",
3835
"MAX_ANGULAR_DEGREE",
3936
"NEIGHBOR_QUADRUPOLE_CHANNELS",
4037
"BispectrumLayout",
@@ -52,5 +49,4 @@
5249
"derive_spin_channels",
5350
"enumerate_degree_triples",
5451
"packed_l2_to_stf",
55-
"validate_charge_state",
5652
]

deepmd/dpmodel/descriptor/dpa4c_nn/charge_state.py

Lines changed: 5 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@
5252
from deepmd.dpmodel.utils.seed import (
5353
child_seed,
5454
)
55+
from deepmd.utils.charge_state import (
56+
CHARGE_OFFSET,
57+
CHARGE_TABLE_ROWS,
58+
MULTIPLICITY_TABLE_ROWS,
59+
)
5560
from deepmd.utils.version import (
5661
check_version_compatibility,
5762
)
@@ -69,78 +74,6 @@
6974
Array,
7075
)
7176

72-
#: Rows of the charge table, covering integer charges in ``[-100, 99]``.
73-
CHARGE_TABLE_ROWS = 200
74-
75-
#: Index of the neutral charge row, so row ``CHARGE_OFFSET + Q`` holds ``Q``.
76-
CHARGE_OFFSET = 100
77-
78-
#: Rows of the spin table, covering integer multiplicities below this bound.
79-
MULTIPLICITY_TABLE_ROWS = 100
80-
81-
#: Half-open range of representable total charges, in units of the elementary
82-
#: charge.
83-
CHARGE_RANGE = (-CHARGE_OFFSET, CHARGE_TABLE_ROWS - CHARGE_OFFSET)
84-
85-
#: Half-open range of representable spin multiplicities.
86-
MULTIPLICITY_RANGE = (0, MULTIPLICITY_TABLE_ROWS)
87-
88-
#: Name of each value of a charge state, in order, for diagnostics.
89-
CHARGE_STATE_FIELDS = ("charge", "multiplicity")
90-
91-
#: Half-open row range addressed by each value of a charge state, in order.
92-
#: A condition is a pair of table row indices, so a host-side boundary that
93-
#: knows these ranges can reject an unaddressable state without knowing which
94-
#: descriptor holds the tables.
95-
CHARGE_STATE_TABLE_RANGES = (CHARGE_RANGE, MULTIPLICITY_RANGE)
96-
97-
98-
def validate_charge_state(charge_spin: Any) -> list[float]:
99-
"""Check that a frame condition addresses a row of each embedding table.
100-
101-
Both tables are indexed directly by the condition, and neither the gather
102-
nor the compiled kernel bounds-checks that index, so an out-of-range value
103-
would read past the table. Every host-side boundary that accepts a charge
104-
state therefore passes it through here first. The per-forward path is
105-
deliberately not guarded: its values come from the data pipeline, which
106-
owns their validity exactly as it owns the validity of an atom type.
107-
108-
Parameters
109-
----------
110-
charge_spin
111-
A pair ``[charge, multiplicity]``, in any sequence form.
112-
113-
Returns
114-
-------
115-
list[float]
116-
The same pair, as two floats.
117-
118-
Raises
119-
------
120-
ValueError
121-
If the pair does not hold exactly two integral values within the
122-
representable ranges.
123-
"""
124-
values = [float(value) for value in np.reshape(np.asarray(charge_spin), (-1,))]
125-
if len(values) != 2:
126-
raise ValueError(
127-
f"A charge state must be a `[charge, multiplicity]` pair, got "
128-
f"{len(values)} values"
129-
)
130-
for value, name, (low, high) in zip(
131-
values,
132-
CHARGE_STATE_FIELDS,
133-
CHARGE_STATE_TABLE_RANGES,
134-
strict=True,
135-
):
136-
if not np.isfinite(value) or value != int(value):
137-
raise ValueError(f"The {name} must be an integer, got {value}")
138-
if not low <= value < high:
139-
raise ValueError(
140-
f"The {name} must lie in [{low}, {high}), got {int(value)}"
141-
)
142-
return values
143-
14477

14578
class ChargeStateEmbedding(NativeOP):
14679
r"""Embed the frame charge and spin multiplicity into two condition vectors.

deepmd/dpmodel/descriptor/make_base_descriptor.py

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -113,17 +113,6 @@ def get_default_chg_spin(self) -> Any:
113113
"""Returns the default charge_spin value, or None."""
114114
return None
115115

116-
def get_chg_spin_table_ranges(self) -> list[tuple[int, int]] | None:
117-
"""Returns the row range each charge_spin value indexes, or None.
118-
119-
A descriptor that embeds the condition by indexing tables reports
120-
one half-open range per value, which makes every acceptable state
121-
an integer tuple inside those ranges. ``None``, the default, means
122-
the condition enters as a continuous quantity and only its width
123-
is constrained.
124-
"""
125-
return None
126-
127116
def get_geo_compress(self) -> bool:
128117
"""Return whether geometric tabulated compression is active.
129118

0 commit comments

Comments
 (0)