Skip to content

Commit eb23860

Browse files
author
Han Wang
committed
fix(pt_expt): bridging is a composition, so standard must reject it
get_standard_model honored `bridging_method` and returned a LinearEnergyModel -- the type asked for was not the type returned. It also made a second owner of the bridging build, and the two owners had already drifted: get_sezm_model promotes descriptor.exclude_types to model-level pair_exclude_types and this one never did, changing a 0.9 A Ni-O dimer by 79.97 eV (max |dF| 318.48 eV/A). Reject `bridging_method` in the standard builder instead. Rejecting rather than ignoring keeps the original fail-loud property: silently dropping the term yields a physically different model than the config requests. DPA4 (`type: "dpa4"`/`"sezm"`) is now the single bridging owner in this backend. The route was unreachable from a validated input.json anyway -- argcheck declares `bridging_method` only under the dpa4/sezm variant -- so no supported configuration changes behavior. Follow-ups: deepmodeling#5947 (drop the exclusion promotion), deepmodeling#5948 (express bridging as an explicit linear_ener composition, after which the restriction is moot).
1 parent 7e539d6 commit eb23860

2 files changed

Lines changed: 101 additions & 55 deletions

File tree

deepmd/pt_expt/model/get_model.py

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -180,10 +180,11 @@ def _compose_bridging(model: Any, data: dict, bridging_method: str) -> Any:
180180
Composition, not a flag (first-principles design): the analytical
181181
bridging term is its own atomic model, summed with the learned one by
182182
the existing linear composition machinery. The ONE owner of the
183-
composition build for this backend -- both :func:`get_sezm_model`
184-
(``type: "dpa4"``) and :func:`get_standard_model` (``type:
185-
"standard"``) route through here, mirroring the dpmodel twin
186-
(``deepmd/dpmodel/model/model.py``).
183+
composition build for this backend: :func:`get_sezm_model`
184+
(``type: "dpa4"``/``"sezm"``) is its only caller, because bridging
185+
yields a composition and so is not expressible on a non-composite
186+
model type -- :func:`get_standard_model` rejects it. Issue #5948
187+
tracks spelling the composition explicitly as ``linear_ener``.
187188
188189
Parameters
189190
----------
@@ -230,31 +231,46 @@ def _compose_bridging(model: Any, data: dict, bridging_method: str) -> Any:
230231

231232

232233
def get_standard_model(data: dict) -> Any:
233-
"""Build a pt_expt standard model, honoring ``bridging_method``.
234+
"""Build a pt_expt standard model: one descriptor plus one fitting net.
234235
235-
pt_expt twin of :func:`deepmd.dpmodel.model.model.get_standard_model`:
236-
the analytical-bridging radii feed the DESCRIPTOR's
237-
InnerClamp/BridgingSwitch and the method composes the atomic model with
238-
its InnerPotential term. Without this wrapper a ``type: "standard"``
239-
config with ``bridging_method`` silently dropped the bridging term
240-
(backend divergence from dpmodel -- issue #5906 Task 4 audit).
236+
``bridging_method`` is rejected here rather than honored. Analytical
237+
bridging is a COMPOSITION -- it yields a ``LinearEnergyModel`` over
238+
``[learned, InnerPotential]`` -- so a builder that accepted it would
239+
return a model of a different kind than the one requested. pt_expt
240+
keeps exactly one bridging owner, :func:`get_sezm_model`
241+
(``type: "dpa4"``/``"sezm"``), so the composition and its
242+
``exclude_types`` reconciliation cannot drift between two builders.
243+
244+
Rejecting is deliberate over silently ignoring: dropping a bridging
245+
term without a word yields a physically different model than the config
246+
asks for. Issue #5948 tracks replacing the flag with an explicit
247+
``linear_ener`` composition, at which point this restriction is moot.
241248
242249
Parameters
243250
----------
244251
data : dict
245252
The data to construct the model.
253+
254+
Returns
255+
-------
256+
Any
257+
The constructed standard model.
258+
259+
Raises
260+
------
261+
ValueError
262+
If ``bridging_method`` is set: bridging is not expressible on a
263+
non-composite model type.
246264
"""
247-
data = copy.deepcopy(data)
248265
bridging_method = str(data.get("bridging_method", "none"))
249-
bridging_enabled = bridging_method.lower() not in ("none", "")
250-
if bridging_enabled:
251-
data.setdefault("descriptor", {})
252-
data["descriptor"]["inner_clamp_r_inner"] = data.get("bridging_r_inner", 0.5)
253-
data["descriptor"]["inner_clamp_r_outer"] = data.get("bridging_r_outer", 0.8)
254-
model = _model_factory.get_standard_model(data)
255-
if not bridging_enabled:
256-
return model
257-
return _compose_bridging(model, data, bridging_method)
266+
if bridging_method.lower() not in ("none", ""):
267+
raise ValueError(
268+
"`bridging_method` is not supported for a standard model in the "
269+
"pt_expt backend: analytical bridging builds a linear "
270+
'composition, not a standard model. Use model `type: "dpa4"` '
271+
'(or `"sezm"`) with the same descriptor and fitting net.'
272+
)
273+
return _model_factory.get_standard_model(data)
258274

259275

260276
def get_native_spin_model(data: dict) -> NativeSpinEnergyModel:

source/tests/pt_expt/model/test_get_model_bridging.py

Lines changed: 64 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,25 @@
11
# SPDX-License-Identifier: LGPL-3.0-or-later
2-
"""pt_expt ``get_standard_model`` must honor ``bridging_method`` like its
3-
dpmodel twin (``deepmd/dpmodel/model/model.py``) -- issue #5906 Task 4
4-
variant-alignment audit, gap 2: a ``type: "standard"`` config with bridging
5-
silently dropped the InnerPotential composition in pt_expt.
2+
"""Analytical bridging has exactly ONE owner per backend.
3+
4+
Bridging builds a COMPOSITION (``LinearEnergyModel`` over
5+
``[learned, InnerPotential]``), so it is not expressible on a non-composite
6+
model type: ``type: "standard"`` would have to return a model of a
7+
different kind than the one requested. pt_expt therefore owns bridging on
8+
the DPA4/SeZM route only and REJECTS it in the standard builder -- loudly,
9+
because silently dropping the term yields a physically different model.
10+
11+
Two builders accepting the flag is exactly how the routes drifted:
12+
``get_sezm_model`` promotes ``descriptor.exclude_types`` to model-level
13+
``pair_exclude_types`` and the standard route never did, which changes a
14+
0.9 A Ni-O dimer by ~80 eV (issue #5947). Issue #5948 replaces the flag
15+
with an explicit ``linear_ener`` composition, after which this restriction
16+
becomes moot.
617
"""
718

819
import copy
920

21+
import pytest
22+
1023
from deepmd.dpmodel.atomic_model.linear_atomic_model import (
1124
LinearEnergyAtomicModel,
1225
)
@@ -42,37 +55,56 @@ def _dpa4_standard_config() -> dict:
4255
}
4356

4457

45-
def test_standard_model_type_builds_bridging_composition() -> None:
46-
"""pt_expt twin of dpmodel model.py's get_standard_model: a config with
47-
bridging_method must compose [learned, InnerPotential], not silently
48-
drop the bridging term.
49-
"""
50-
data = _dpa4_standard_config()
58+
def _bridged(data: dict) -> dict:
5159
data["bridging_method"] = "ZBL"
5260
data["bridging_r_inner"] = 0.8
5361
data["bridging_r_outer"] = 1.2
54-
model = get_standard_model(copy.deepcopy(data))
55-
assert isinstance(model.atomic_model, LinearEnergyAtomicModel)
56-
assert len(model.atomic_model.models) == 2
57-
# The descriptor radii injection must ride the same seam (a composition
58-
# without the inner-clamp radii would be a half-applied bridging config):
59-
desc = model.atomic_model.models[0].descriptor
60-
assert desc.bridging_switch is not None
61-
# And the get_model router (type omitted -> "standard") reaches the same
62-
# composition:
63-
routed = get_model(copy.deepcopy(data))
64-
assert isinstance(routed.atomic_model, LinearEnergyAtomicModel)
65-
66-
67-
def test_standard_model_type_maps_dpa4_fitting() -> None:
68-
"""Dpmodel maps dpa4_ener/sezm_ener fitting under type:'standard' via the
69-
model registry; pt_expt must not raise where dpmodel builds.
62+
return data
63+
64+
65+
def test_standard_builder_rejects_bridging() -> None:
66+
"""The standard builder must not hand back a composition."""
67+
with pytest.raises(ValueError, match="bridging_method"):
68+
get_standard_model(_bridged(_dpa4_standard_config()))
69+
70+
71+
def test_get_model_rejects_bridging_without_dpa4_model_type() -> None:
72+
"""Same contract through the dispatcher: an omitted model type defaults
73+
to the standard route, so it must reject rather than compose.
74+
"""
75+
with pytest.raises(ValueError, match="bridging_method"):
76+
get_model(_bridged(_dpa4_standard_config()))
77+
78+
79+
def test_standard_builder_without_bridging_is_unaffected() -> None:
80+
"""The rejection keys on the flag, not on the DPA4 components: a plain
81+
DPA4 standard model still builds and carries no bridging switch.
7082
"""
7183
model = get_standard_model(_dpa4_standard_config())
72-
assert model is not None
84+
assert not isinstance(model.atomic_model, LinearEnergyAtomicModel)
7385
assert model.atomic_model.descriptor.bridging_switch is None
7486

7587

88+
@pytest.mark.parametrize(
89+
"model_type",
90+
[
91+
"dpa4", # canonical spelling
92+
"sezm", # pt-compatible alias
93+
],
94+
)
95+
def test_dpa4_model_type_owns_the_composition(model_type: str) -> None:
96+
"""The one supported spelling composes [learned, InnerPotential] and
97+
injects the radii into the learned child's descriptor.
98+
"""
99+
data = _bridged(_dpa4_standard_config())
100+
data["type"] = model_type
101+
model = get_model(copy.deepcopy(data))
102+
assert isinstance(model.atomic_model, LinearEnergyAtomicModel)
103+
assert len(model.atomic_model.models) == 2
104+
# a composition without the inner-clamp radii would be half-applied
105+
assert model.atomic_model.models[0].descriptor.bridging_switch is not None
106+
107+
76108
def test_pt_checkpoint_eval_works_for_composition(tmp_path) -> None:
77109
"""``DeepEval`` on a ``.pt`` checkpoint of a bridging composition.
78110
@@ -91,11 +123,8 @@ def test_pt_checkpoint_eval_works_for_composition(tmp_path) -> None:
91123
ModelWrapper,
92124
)
93125

94-
config = _dpa4_standard_config()
126+
config = _bridged(_dpa4_standard_config())
95127
config["type"] = "dpa4"
96-
config["bridging_method"] = "ZBL"
97-
config["bridging_r_inner"] = 0.8
98-
config["bridging_r_outer"] = 1.2
99128
model = get_model(copy.deepcopy(config)).to(torch.float64).eval()
100129
ckpt = str(tmp_path / "dpa4_zbl.pt")
101130
wrapper = ModelWrapper(model, model_params=copy.deepcopy(config))
@@ -122,8 +151,9 @@ def test_compile_attention_probe_tolerates_composition() -> None:
122151
_warn_compiled_attention,
123152
)
124153

125-
data = _dpa4_standard_config()
126-
data["bridging_method"] = "ZBL"
127-
model = get_standard_model(data)
154+
data = _bridged(_dpa4_standard_config())
155+
data["type"] = "dpa4"
156+
model = get_model(data)
157+
assert isinstance(model.atomic_model, LinearEnergyAtomicModel)
128158
# must not raise
129159
_warn_compiled_attention(model, "Default")

0 commit comments

Comments
 (0)