Skip to content

Commit e48b6e2

Browse files
author
Han Wang
committed
fix: refresh the ZBL lookup on change_type_map; allow native spin + bridging
Two review items. **3649295675 -- ZBL element lookup went stale on change_type_map.** ``InterPotentialAtomicModel`` inherited the generic ``BaseAtomicModel.change_type_map``, which rewrites the public map and the stat/exclusion state only, so ``InterPotential`` kept its ORIGINAL nuclear charges while ``atype`` values already meant the new elements -- silently wrong energies after a reorder, ``IndexError`` after adding a type, and a restart-time jump because serialization records the new map. The lookup belongs to ``InterPotential``, so it owns the update: a new ``change_type_map`` rebuilds symbols, count and table together from the symbols (not by permuting indices), keeping the array's namespace/dtype/ device so a pt_expt buffer -- possibly on CUDA -- is updated in place. The atomic model delegates rather than reimplementing. **3649276109 -- native spin + analytical bridging was rejected.** The combination needs no new machinery: ``get_standard_model`` already OWNS assembling the atomic model, bridging composition included, so the guard is gone and ``get_native_spin_model`` now delegates to it and re-classes whatever atomic model comes back -- a single learned model or a ``LinearEnergyAtomicModel`` over ``[learned, InterPotential]``. The learned child consumes ``spin``; the analytical child accepts and ignores it. Two supporting changes: a shared ``learned_descriptor()`` so the descriptor capability gate reads the learned child either way, and ``NSM.serialize`` now records the backbone's own wire type (``backbone_type``) -- it was hard-coded to "standard", which fails on a composed backbone whose dict has a different shape and @Version. Archives without the field still read as "standard". Also fixes the C++ pair-exclusion regression added in ecfb7a8: its ``cpu_lmp_nlist`` case compared an nghost=0 explicit-nlist run (no periodic images) against the PBC reference; it now uses the gas-phase one, the same convention test_deepspin_dpa4_graph_ptexpt.cc follows. Tests: change_type_map reorder / add / drop / serialize-roundtrip in both dpmodel and pt_expt (the pt_expt twin asserts the rebuilt lookup is still a torch buffer on the model device); native spin + ZBL construction, eager energy/force/force_mag with mask_mag, the analytical term checked against an independent in-test pair loop at 1e-10, serialization, and graph freeze + DeepPot parity at 1e-10. The stale ``test_native_spin_with_bridging_fails_fast`` is replaced by those.
1 parent fd6fa77 commit e48b6e2

7 files changed

Lines changed: 549 additions & 41 deletions

File tree

deepmd/dpmodel/atomic_model/inter_potential.py

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,64 @@ def __init__(self, type_map: list[str], mode: str = "zbl") -> None:
9797
self.mode = mode
9898
self.type_map = list(type_map)
9999
self.ntypes_real = len(type_map)
100+
self.atomic_numbers = self._lookup_from_type_map(type_map)
101+
102+
@staticmethod
103+
def _lookup_from_type_map(type_map: list[str], like: Array | None = None) -> Array:
104+
"""Build the per-type nuclear-charge lookup from element symbols.
105+
106+
Parameters
107+
----------
108+
type_map : list[str]
109+
Element symbols; index corresponds to ``atype`` values.
110+
like : Array, optional
111+
When given, the result is created in this array's namespace, dtype
112+
and device instead of NumPy -- so an in-place rebuild on a wrapped
113+
backend (pt_expt buffer, possibly on CUDA) stays where it was.
114+
115+
Returns
116+
-------
117+
Array
118+
Nuclear charges, shape ``(len(type_map),)``.
119+
120+
Raises
121+
------
122+
ValueError
123+
If an element symbol is not in :data:`ELEMENT_TO_Z`.
124+
"""
100125
atomic_numbers = []
101126
for elem in type_map:
102127
z = ELEMENT_TO_Z.get(elem)
103128
if z is None:
104129
raise ValueError(f"Unknown element symbol: {elem}")
105130
atomic_numbers.append(z)
106-
self.atomic_numbers = np.asarray(atomic_numbers, dtype=np.float64)
131+
arr = np.asarray(atomic_numbers, dtype=np.float64)
132+
if like is None:
133+
return arr
134+
xp = array_api_compat.array_namespace(like)
135+
return xp.asarray(arr, dtype=like.dtype, device=array_api_compat.device(like))
136+
137+
def change_type_map(self, type_map: list[str]) -> None:
138+
"""Rebuild the element lookup for a new type map.
139+
140+
THIS OWNS the element lookup, so it owns every update of it: the
141+
symbols, their count and the nuclear-charge table are one piece of
142+
state and are replaced together. Reordering, adding and dropping
143+
elements are all covered -- the table is rebuilt from the symbols
144+
rather than permuted, so no index bookkeeping can drift. The rebuilt
145+
array keeps the current one's namespace/dtype/device, so a wrapped
146+
backend (pt_expt buffer on CPU or CUDA) is updated in place.
147+
148+
Parameters
149+
----------
150+
type_map : list[str]
151+
The new element symbols.
152+
"""
153+
self.atomic_numbers = self._lookup_from_type_map(
154+
type_map, like=self.atomic_numbers
155+
)
156+
self.type_map = list(type_map)
157+
self.ntypes_real = len(type_map)
107158

108159
@staticmethod
109160
def _zbl_pair_energy(xp: Any, r: Array, zi: Array, zj: Array) -> Array:
@@ -259,6 +310,31 @@ def __init__(
259310
)
260311
super().init_out_stat()
261312

313+
def change_type_map(
314+
self, type_map: list[str], model_with_new_type_stat: Any | None = None
315+
) -> None:
316+
"""Change the type related params to new ones, according to `type_map` and the original one in the model.
317+
If there are new types in `type_map`, statistics will be updated accordingly to `model_with_new_type_stat` for these new types.
318+
319+
The generic base handles the public map and the stat/exclusion state;
320+
the element lookup belongs to :class:`InterPotential`, so the update is
321+
delegated there rather than reimplemented here (review 3649295675 --
322+
without it the lookup keeps the ORIGINAL elements while ``atype``
323+
values mean new ones, and a longer new map raises ``IndexError``).
324+
325+
Parameters
326+
----------
327+
type_map : list[str]
328+
The new element symbols.
329+
model_with_new_type_stat : optional
330+
Model with statistics for the new types (unused: an analytical
331+
term has no fitted statistics).
332+
"""
333+
super().change_type_map(
334+
type_map, model_with_new_type_stat=model_with_new_type_stat
335+
)
336+
self.potential.change_type_map(type_map)
337+
262338
def fitting_output_def(self) -> FittingOutputDef:
263339
"""Per-atom analytical energy: reducible and fully differentiable."""
264340
return FittingOutputDef(

deepmd/dpmodel/model/model.py

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,36 @@ def get_spin_model(data: dict) -> SpinModel:
240240
return SpinModel(backbone_model=backbone_model, spin=spin)
241241

242242

243+
def learned_descriptor(atomic_model: Any) -> Any:
244+
"""The descriptor of the LEARNED term of an atomic model.
245+
246+
A plain atomic model owns its descriptor directly; a composition (e.g.
247+
analytical bridging, ``LinearEnergyAtomicModel`` over ``[learned,
248+
InterPotential]``) has exactly one descriptor-bearing child, since the
249+
analytical terms are descriptor-free. Shared by every backend's builder
250+
so descriptor capability gates read the same object regardless of whether
251+
the model happens to be composed.
252+
253+
Parameters
254+
----------
255+
atomic_model
256+
The atomic model to inspect.
257+
258+
Returns
259+
-------
260+
descriptor or None
261+
The learned descriptor, or ``None`` if no child has one.
262+
"""
263+
descriptor = getattr(atomic_model, "descriptor", None)
264+
if descriptor is not None:
265+
return descriptor
266+
for child in getattr(atomic_model, "models", []):
267+
descriptor = getattr(child, "descriptor", None)
268+
if descriptor is not None:
269+
return descriptor
270+
return None
271+
272+
243273
def get_native_spin_model(data: dict) -> NativeSpinEnergyModel:
244274
"""Get a native (virtual-atom-free) spin model from a dictionary.
245275
@@ -250,19 +280,22 @@ def get_native_spin_model(data: dict) -> NativeSpinEnergyModel:
250280
eligible; the gate is the capability method, not a descriptor-type
251281
list.
252282
283+
The non-spin backbone is built by :func:`get_standard_model`, which OWNS
284+
everything about assembling the atomic model -- descriptor/fitting,
285+
exclusions and the analytical-bridging composition -- so ``spin`` and
286+
``bridging_method`` combine for free: the wrapper re-classes whatever
287+
atomic model came back, be it a single learned model or a
288+
``LinearEnergyAtomicModel`` over ``[learned, InterPotential]`` (the
289+
analytical child accepts and ignores ``spin``; the learned child consumes
290+
it).
291+
253292
Parameters
254293
----------
255294
data : dict
256295
The data to construct the model.
257296
"""
258297
data = copy.deepcopy(data)
259298
spin_cfg = data.pop("spin")
260-
if str(data.get("bridging_method", "none")).lower() not in ("none", ""):
261-
raise NotImplementedError(
262-
"analytical bridging combined with the native spin scheme is a "
263-
"follow-up (the bridged model is a linear composition; the "
264-
"native-spin factory composes over a single standard model)"
265-
)
266299
# Expand index/symbol forms of ``use_spin`` against ``type_map`` into the
267300
# per-type boolean list (pure; validates symbols).
268301
use_spin = normalize_spin_use_spin(spin_cfg["use_spin"], data["type_map"])
@@ -271,10 +304,10 @@ def get_native_spin_model(data: dict) -> NativeSpinEnergyModel:
271304
virtual_scale=spin_cfg.get("virtual_scale", 1.0),
272305
allow_missing_label=spin_cfg.get("allow_missing_label", False),
273306
)
307+
data.setdefault("descriptor", {})
274308
data["descriptor"]["use_spin"] = use_spin
275-
ntypes = len(data["type_map"])
276309
try:
277-
descriptor, fitting, _ = _get_standard_model_components(data, ntypes)
310+
backbone_model = get_standard_model(data)
278311
except TypeError as err:
279312
if "use_spin" not in str(err):
280313
# Unrelated construction error (e.g. a bogus fitting kwarg):
@@ -289,19 +322,13 @@ def get_native_spin_model(data: dict) -> NativeSpinEnergyModel:
289322
"support (supports_native_spin()); descriptor type "
290323
f"{data['descriptor'].get('type')!r} does not accept `use_spin`"
291324
) from err
292-
if not descriptor.supports_native_spin():
325+
descriptor = learned_descriptor(backbone_model.atomic_model)
326+
if descriptor is None or not descriptor.supports_native_spin():
293327
raise NotImplementedError(
294328
"spin scheme 'native' requires a descriptor declaring "
295329
"supports_native_spin()"
296330
)
297-
return NativeSpinEnergyModel(
298-
descriptor=descriptor,
299-
fitting=fitting,
300-
type_map=data["type_map"],
301-
atom_exclude_types=data.get("atom_exclude_types", []),
302-
pair_exclude_types=data.get("pair_exclude_types", []),
303-
spin=spin,
304-
)
331+
return NativeSpinEnergyModel(atomic_model_=backbone_model.atomic_model, spin=spin)
305332

306333

307334
def get_model(data: dict) -> BaseModel:

deepmd/dpmodel/model/native_spin_model.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,12 @@ def call(
227227

228228
def serialize(self) -> dict:
229229
data = super().serialize()
230+
# The backbone's own wire type would be lost under "native_spin";
231+
# keep it so deserialize can rebuild the RIGHT backbone. It is
232+
# not always "standard": with analytical bridging the backbone is
233+
# a composition ("linear"), whose dict has a different shape and
234+
# @version.
235+
data["backbone_type"] = data.get("type", "standard")
230236
data["type"] = "native_spin"
231237
data["spin"] = self.spin.serialize()
232238
return data
@@ -236,11 +242,18 @@ def deserialize(cls, data: dict) -> "NSM":
236242
data = data.copy()
237243
data.pop("type", None)
238244
spin = Spin.deserialize(data.pop("spin"))
239-
# make_model flat shape: the remaining dict IS the standard
240-
# model (atomic) dict -- its @class/@version belong to the
241-
# atomic deserialize and must stay.
242-
data["type"] = "standard"
243-
backbone = T_Model.deserialize(data)
245+
# make_model flat shape: the remaining dict IS the backbone
246+
# (atomic) dict -- its @class/@version belong to the backbone's
247+
# deserialize and must stay. Archives written before
248+
# ``backbone_type`` existed are all plain standard models.
249+
backbone_type = data.pop("backbone_type", "standard")
250+
data["type"] = backbone_type
251+
backbone_cls = (
252+
T_Model
253+
if backbone_type == "standard"
254+
else T_Model.get_class_by_type(backbone_type)
255+
)
256+
backbone = backbone_cls.deserialize(data)
244257
return cls(atomic_model_=backbone.atomic_model, spin=spin)
245258

246259
return NSM

deepmd/pt_expt/model/get_model.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -271,12 +271,6 @@ def get_native_spin_model(data: dict) -> NativeSpinEnergyModel:
271271
data = copy.deepcopy(data)
272272
spin_cfg = data.pop("spin")
273273
data.setdefault("descriptor", {})
274-
if str(data.get("bridging_method", "none")).lower() not in ("none", ""):
275-
raise NotImplementedError(
276-
"analytical bridging combined with the native spin scheme is a "
277-
"follow-up (the bridged model is a linear composition; the "
278-
"native-spin factory composes over a single standard model)"
279-
)
280274
# Expand index/symbol forms of ``use_spin`` against ``type_map`` into the
281275
# per-type boolean list (pure; validates symbols).
282276
use_spin = normalize_spin_use_spin(spin_cfg["use_spin"], data["type_map"])
@@ -306,8 +300,14 @@ def get_native_spin_model(data: dict) -> NativeSpinEnergyModel:
306300
"support (supports_native_spin()); descriptor type "
307301
f"{data['descriptor'].get('type')!r} does not accept `use_spin`"
308302
) from err
309-
descriptor = backbone_model.atomic_model.descriptor
310-
if not descriptor.supports_native_spin():
303+
# A bridged backbone is a LinearEnergyAtomicModel; the capability gate
304+
# reads the LEARNED child's descriptor either way (shared helper).
305+
from deepmd.dpmodel.model.model import (
306+
learned_descriptor,
307+
)
308+
309+
descriptor = learned_descriptor(backbone_model.atomic_model)
310+
if descriptor is None or not descriptor.supports_native_spin():
311311
raise NotImplementedError(
312312
"spin scheme 'native' requires a descriptor declaring "
313313
"supports_native_spin()"

source/api_cc/tests/test_deepspin_dpa4_pairexcl_ptexpt.cc

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,18 @@ class TestInferDeepSpinDpa4PairExclPtExpt : public ::testing::Test {
7272
std::vector<VALUETYPE> expected_f;
7373
std::vector<VALUETYPE> expected_fm;
7474
std::vector<VALUETYPE> expected_tot_v;
75+
// NoPBC twin: an explicit nghost=0 InputNlist carries no periodic images,
76+
// so the LAMMPS-nlist case below is the gas-phase system and must be
77+
// compared against the gas-phase reference (same convention as
78+
// test_deepspin_dpa4_graph_ptexpt.cc, which runs cpu_lmp_nlist only in its
79+
// Nopbc fixture).
80+
std::vector<VALUETYPE> expected_e_nopbc;
81+
std::vector<VALUETYPE> expected_f_nopbc;
82+
std::vector<VALUETYPE> expected_fm_nopbc;
7583

7684
int natoms;
7785
double expected_tot_e;
86+
double expected_tot_e_nopbc;
7887

7988
deepmd::DeepSpin dp_excl;
8089
deepmd::DeepSpin dp_base;
@@ -98,14 +107,19 @@ class TestInferDeepSpinDpa4PairExclPtExpt : public ::testing::Test {
98107
expected_f = ref.get<VALUETYPE>("pbc", "expected_f");
99108
expected_fm = ref.get<VALUETYPE>("pbc", "expected_fm");
100109
expected_tot_v = ref.get<VALUETYPE>("pbc", "expected_tot_v");
110+
expected_e_nopbc = ref.get<VALUETYPE>("nopbc", "expected_e");
111+
expected_f_nopbc = ref.get<VALUETYPE>("nopbc", "expected_f");
112+
expected_fm_nopbc = ref.get<VALUETYPE>("nopbc", "expected_fm");
101113

102114
natoms = expected_e.size();
103115
EXPECT_EQ(natoms * 3, expected_f.size());
104116
EXPECT_EQ(natoms * 3, expected_fm.size());
105117
EXPECT_EQ(9, expected_tot_v.size());
106118
expected_tot_e = 0.;
119+
expected_tot_e_nopbc = 0.;
107120
for (int ii = 0; ii < natoms; ++ii) {
108121
expected_tot_e += expected_e[ii];
122+
expected_tot_e_nopbc += expected_e_nopbc[ii];
109123
}
110124
};
111125

@@ -155,11 +169,13 @@ TYPED_TEST(TestInferDeepSpinDpa4PairExclPtExpt, cpu_lmp_nlist) {
155169
const std::vector<VALUETYPE>& coord = this->coord;
156170
const std::vector<VALUETYPE>& spin = this->spin;
157171
std::vector<int>& atype = this->atype;
158-
std::vector<VALUETYPE>& box = this->box;
159-
std::vector<VALUETYPE>& expected_f = this->expected_f;
160-
std::vector<VALUETYPE>& expected_fm = this->expected_fm;
172+
std::vector<VALUETYPE>& expected_f = this->expected_f_nopbc;
173+
std::vector<VALUETYPE>& expected_fm = this->expected_fm_nopbc;
161174
int& natoms = this->natoms;
162-
double& expected_tot_e = this->expected_tot_e;
175+
double& expected_tot_e = this->expected_tot_e_nopbc;
176+
// An nghost=0 InputNlist carries no periodic images, so this is the
177+
// gas-phase system: empty box + the NoPBC reference.
178+
std::vector<VALUETYPE> box = {};
163179

164180
double ener;
165181
std::vector<VALUETYPE> force, force_mag, virial;

0 commit comments

Comments
 (0)