Skip to content

Commit 2f4e63d

Browse files
author
Han Wang
committed
refactor: the atomic model answers supports_native_spin, not a descriptor hunt
The native-spin builders reached into the atomic model for a descriptor (``learned_descriptor``) to decide spin eligibility -- duck-typing that assumes a descriptor+fitting architecture, guesses which child of a composition is the learned one, and cannot answer at all for an atomic model with no descriptor. Same mistake the ``supports_graph_route`` detour made: the capability belongs ON the atomic model. ``supports_native_spin()`` is now the twin of ``uses_graph_lower()``: concrete ``False`` on ``BaseAtomicModel``; ``DPAtomicModel`` delegates to its descriptor; ``LinearEnergyAtomicModel`` answers from its children. Both builders just ask the atomic model, and ``learned_descriptor`` is gone. ``DPAtomicModel`` already cached the answer, but as a bool ATTRIBUTE -- which is exactly why reaching for the descriptor looked necessary, since an attribute cannot be overridden polymorphically by the base or by a composition. It becomes a method backed by the private ``_supports_native_spin``, so the per-forward hot path still avoids a descriptor call. The composition uses ANY, deliberately unlike ``uses_graph_lower``'s ALL: every child must run on the shared graph, but spin only has to reach ONE consumer -- analytical terms accept and ignore it. With no consumer the magnetic force is identically zero, which is not a spin model; pinned by a test that a two-ZBL composition reports False.
1 parent e48b6e2 commit 2f4e63d

8 files changed

Lines changed: 83 additions & 51 deletions

File tree

deepmd/dpmodel/atomic_model/base_atomic_model.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,17 @@ def uses_graph_lower(self) -> bool:
179179
"""
180180
return False
181181

182+
def supports_native_spin(self) -> bool:
183+
"""Returns whether this atomic model consumes a per-atom spin input.
184+
185+
Generic capability (concrete default ``False``), the twin of
186+
:meth:`uses_graph_lower`: the model layer asks the atomic model
187+
directly instead of reaching into it for a descriptor, so the answer
188+
stays correct for architectures with no descriptor at all (analytical
189+
terms) and for compositions.
190+
"""
191+
return False
192+
182193
def get_default_fparam(self) -> list[float] | None:
183194
"""Get the default frame parameters."""
184195
return None

deepmd/dpmodel/atomic_model/dp_atomic_model.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ def __init__(
133133
# (that would be a ``TypeError``, not a no-op). Queried via the
134134
# ``supports_native_spin`` capability method declared on
135135
# ``BaseDescriptor`` (concrete default ``False``; DPA4 overrides).
136-
self.supports_native_spin: bool = self.descriptor.supports_native_spin()
136+
self._supports_native_spin: bool = self.descriptor.supports_native_spin()
137137
# Same capability method as ``supports_native_spin`` above, for the
138138
# frame-level ``charge_spin`` FiLM kwarg: only DPA4's ``call_graph``
139139
# declares it; other descriptors' ``call_graph`` would ``TypeError``
@@ -167,6 +167,10 @@ def uses_graph_lower(self) -> bool:
167167
"""Delegates to this model's own descriptor."""
168168
return bool(self.descriptor.uses_graph_lower())
169169

170+
def supports_native_spin(self) -> bool:
171+
"""Delegates to this model's own descriptor (cached at construction)."""
172+
return self._supports_native_spin
173+
170174
def fitting_output_def(self) -> FittingOutputDef:
171175
"""Get the output def of the fitting net."""
172176
return self.fitting_net.output_def()
@@ -375,7 +379,7 @@ def forward_atomic_graph(
375379
# See ``self.supports_native_spin``/``self.supports_charge_spin`` in
376380
# ``__init__``: only forward the ``spin``/``charge_spin`` keyword to
377381
# descriptors whose ``call_graph`` declares it.
378-
spin_kwargs = {"spin": spin} if self.supports_native_spin else {}
382+
spin_kwargs = {"spin": spin} if self._supports_native_spin else {}
379383
charge_spin_kwargs = (
380384
{"charge_spin": charge_spin} if self.supports_charge_spin else {}
381385
)

deepmd/dpmodel/atomic_model/linear_atomic_model.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,17 @@ def uses_graph_lower(self) -> bool:
261261
"""
262262
return all(m.uses_graph_lower() for m in self.models)
263263

264+
def supports_native_spin(self) -> bool:
265+
"""Spin-capable when ANY child consumes the spin input.
266+
267+
Unlike :meth:`uses_graph_lower` (every child must run on the shared
268+
graph), spin only has to reach ONE consumer: analytical terms accept
269+
and ignore it, so a composition of a spin-aware learned model with a
270+
ZBL term is a valid native-spin model. With no consumer at all the
271+
magnetic force would be identically zero, which is not a spin model.
272+
"""
273+
return any(m.supports_native_spin() for m in self.models)
274+
264275
def forward_atomic_graph(
265276
self,
266277
graph: Any,

deepmd/dpmodel/model/model.py

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -240,44 +240,14 @@ 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-
273243
def get_native_spin_model(data: dict) -> NativeSpinEnergyModel:
274244
"""Get a native (virtual-atom-free) spin model from a dictionary.
275245
276246
Unlike :func:`get_spin_model`, no virtual atoms or doubled type map are
277247
introduced: ``spin`` is injected into the descriptor config as
278248
``use_spin`` and consumed by the descriptor's equivariant spin
279-
embedding. Any descriptor declaring ``supports_native_spin()`` is
280-
eligible; the gate is the capability method, not a descriptor-type
249+
embedding. Any atomic model declaring ``supports_native_spin()`` is
250+
eligible; the gate is that capability method, not a descriptor-type
281251
list.
282252
283253
The non-spin backbone is built by :func:`get_standard_model`, which OWNS
@@ -322,10 +292,12 @@ def get_native_spin_model(data: dict) -> NativeSpinEnergyModel:
322292
"support (supports_native_spin()); descriptor type "
323293
f"{data['descriptor'].get('type')!r} does not accept `use_spin`"
324294
) from err
325-
descriptor = learned_descriptor(backbone_model.atomic_model)
326-
if descriptor is None or not descriptor.supports_native_spin():
295+
# The ATOMIC MODEL answers the capability -- it knows its own structure,
296+
# so this holds for a plain descriptor+fitting model and for a bridging
297+
# composition alike, with no assumption here about either.
298+
if not backbone_model.atomic_model.supports_native_spin():
327299
raise NotImplementedError(
328-
"spin scheme 'native' requires a descriptor declaring "
300+
"spin scheme 'native' requires an atomic model declaring "
329301
"supports_native_spin()"
330302
)
331303
return NativeSpinEnergyModel(atomic_model_=backbone_model.atomic_model, spin=spin)

deepmd/pt_expt/model/get_model.py

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,9 @@ def get_native_spin_model(data: dict) -> NativeSpinEnergyModel:
258258
for the DPA4/SeZM family (keeping its bridging/lora/compile/
259259
preset_out_bias rejections and ``exclude_types`` consistency check),
260260
else :func:`get_standard_model` -- then re-classed through the
261-
registered :class:`NativeSpinEnergyModel`. Eligibility is the
262-
``descriptor.supports_native_spin()`` capability, not a descriptor-type
263-
list.
261+
registered :class:`NativeSpinEnergyModel`. Eligibility is the atomic
262+
model's own ``supports_native_spin()`` capability, not a descriptor-type
263+
list -- so a bridging composition answers for itself.
264264
265265
Parameters
266266
----------
@@ -300,16 +300,12 @@ def get_native_spin_model(data: dict) -> NativeSpinEnergyModel:
300300
"support (supports_native_spin()); descriptor type "
301301
f"{data['descriptor'].get('type')!r} does not accept `use_spin`"
302302
) from err
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():
303+
# The ATOMIC MODEL answers the capability -- it knows its own structure,
304+
# so this holds for a plain descriptor+fitting model and for a bridging
305+
# composition alike, with no assumption here about either.
306+
if not backbone_model.atomic_model.supports_native_spin():
311307
raise NotImplementedError(
312-
"spin scheme 'native' requires a descriptor declaring "
308+
"spin scheme 'native' requires an atomic model declaring "
313309
"supports_native_spin()"
314310
)
315311
return NativeSpinEnergyModel(atomic_model_=backbone_model.atomic_model, spin=spin)

source/tests/common/dpmodel/test_dp_atomic_model.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ def test_methods(self) -> None:
5858
# Base-default (False) branch of the descriptor spin capabilities,
5959
# cached at construction via direct method calls (the True branch is
6060
# pinned in test_dpa4_call_graph.py).
61-
self.assertFalse(md0.supports_native_spin)
61+
self.assertFalse(md0.supports_native_spin())
6262
self.assertFalse(md0.supports_charge_spin)
6363

6464
def test_self_consistency(

source/tests/common/dpmodel/test_dpa4_call_graph.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ def test_supports_native_spin_capability_gate() -> None:
424424
seed=5,
425425
)
426426
dpa4_model = DPAtomicModel(dd, ft, type_map=["A", "B", "C"])
427-
assert dpa4_model.supports_native_spin is True
427+
assert dpa4_model.supports_native_spin() is True
428428
assert dpa4_model.supports_charge_spin is True
429429

430430

source/tests/common/dpmodel/test_zbl_bridging.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,3 +312,41 @@ def test_serialize_roundtrip_after_change_type_map(self) -> None:
312312
np.testing.assert_allclose(
313313
_pair_energy(restored), _pair_energy(model), rtol=1e-12
314314
)
315+
316+
317+
class TestNativeSpinCapabilityOnAtomicModel:
318+
"""``supports_native_spin`` is answered by the ATOMIC MODEL.
319+
320+
The model layer must not reach into an atomic model for a descriptor to
321+
decide spin eligibility: an analytical term has no descriptor at all, and
322+
a composition has several children. Each atomic model answers from its
323+
own structure, exactly like ``uses_graph_lower``.
324+
"""
325+
326+
def test_analytical_term_is_not_spin_capable(self) -> None:
327+
zbl = InterPotentialAtomicModel(type_map=["Ni", "O"], rcut=4.0, sel=[8])
328+
# inherits the concrete base default -- no descriptor, no spin input
329+
assert zbl.supports_native_spin() is False
330+
331+
def test_composition_is_capable_when_any_child_is(self) -> None:
332+
"""ANY, not ALL: analytical children accept and ignore ``spin``."""
333+
learned = get_model(
334+
{
335+
**copy.deepcopy(ZBL_CONFIG),
336+
"spin": {"use_spin": [True, False], "scheme": "native"},
337+
}
338+
).atomic_model
339+
assert learned.supports_native_spin() is True
340+
kinds = [type(c).__name__ for c in learned.models]
341+
assert kinds[1] == "InterPotentialAtomicModel", kinds
342+
# ... and the spin-free analytical child alone is not capable
343+
assert learned.models[1].supports_native_spin() is False
344+
345+
def test_composition_without_a_spin_consumer_is_not_capable(self) -> None:
346+
"""No consumer => the magnetic force would be identically zero."""
347+
zbl_a = InterPotentialAtomicModel(type_map=["Ni", "O"], rcut=4.0, sel=[8])
348+
zbl_b = InterPotentialAtomicModel(type_map=["Ni", "O"], rcut=4.0, sel=[8])
349+
composed = LinearEnergyAtomicModel(
350+
[zbl_a, zbl_b], type_map=["Ni", "O"], weights="sum"
351+
)
352+
assert composed.supports_native_spin() is False

0 commit comments

Comments
 (0)