Skip to content

Commit fa46570

Browse files
author
Han Wang
committed
fix(pt_expt): keep use_amp out of serialization, fix the assembly boundary
use_amp is a runtime/training policy, not model state: revert the dpa4/sezm serialize additions (a use_amp record leaks a torch runtime option into cross-backend records -- the jax deserializer rejects use_amp=true -- and deepmodeling#5963 established that checkpoints must not carry the AMP switch). The real pt_expt bug is in model assembly: make_model handed the raw dpmodel atomic class to the dpmodel CM, so the constructed atomic model was converted through the auto-wrap serialize()/deserialize() round-trip and every runtime-only option on the live descriptor was reset to its constructor default. Hand the CM the auto-wrapped atomic class instead: the atomic model is constructed directly as a torch module and the live (already wrapped) descriptor/fitting are kept as-is -- no round-trip. Regression tests exercise the public construction path (get_model with descriptor.use_amp=false) and pin that the portable record does not carry use_amp.
1 parent 63071be commit fa46570

6 files changed

Lines changed: 108 additions & 36 deletions

File tree

deepmd/dpmodel/descriptor/dpa4.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2806,11 +2806,6 @@ def serialize(self) -> dict[str, Any]:
28062806
"mlp_bias": self.mlp_bias,
28072807
"exclude_types": self.exclude_types,
28082808
"eps": self.eps,
2809-
# Must round-trip: pt_expt rebuilds the descriptor from this
2810-
# dict, so omitting the key silently reset a configured
2811-
# ``use_amp: false`` to True and kept training in bfloat16.
2812-
# Older records without it still load (__init__ defaults it).
2813-
"use_amp": self.use_amp,
28142809
"trainable": self.trainable,
28152810
"seed": self.seed,
28162811
"inner_clamp_r_inner": self.inner_clamp_r_inner,

deepmd/pt/model/descriptor/sezm.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2556,9 +2556,6 @@ def serialize(self) -> dict[str, Any]:
25562556
"mlp_bias": self.mlp_bias,
25572557
"exclude_types": self.exclude_types,
25582558
"eps": self.eps,
2559-
# Kept in step with the dpmodel serialize contract so both
2560-
# backends' records carry the same keys.
2561-
"use_amp": self.use_amp,
25622559
"trainable": self.trainable,
25632560
"seed": self.seed,
25642561
"inner_clamp_r_inner": self.inner_clamp_r_inner,

deepmd/pt_expt/common.py

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -136,32 +136,54 @@ def try_convert_module(value: Any) -> torch.nn.Module | None:
136136
_AUTO_WRAPPED_CLASSES: dict[type, type] = {}
137137

138138

139-
def _auto_wrap_native_op(value: NativeOP) -> torch.nn.Module:
140-
"""Auto-wrap any NativeOP as a torch.nn.Module via ``torch_module``.
139+
def auto_wrapped_class(cls: type) -> type:
140+
"""Return the cached ``torch_module`` auto-wrap of a dpmodel class.
141141
142142
Creates a subclass with a generic ``forward`` that delegates to ``call``,
143143
then applies ``torch_module`` to get full ``__setattr__`` / post-init
144144
list conversion. The wrapped class is cached per dpmodel type.
145145
146+
Constructing this class DIRECTLY (instead of building the raw dpmodel
147+
class and converting the instance afterwards) is the lossless path:
148+
instance conversion round-trips through ``serialize()``/``deserialize()``,
149+
which by design drops runtime-only configuration (e.g. the DPA4
150+
``use_amp`` switch) from the portable record.
151+
146152
Parameters
147153
----------
148-
value : NativeOP
149-
The dpmodel object to wrap.
154+
cls : type
155+
The dpmodel NativeOP class to wrap.
150156
151157
Returns
152158
-------
153-
torch.nn.Module
154-
The wrapped pt_expt module, deserialized from value's serialized state.
159+
type
160+
The ``torch_module``-wrapped subclass.
155161
"""
156-
cls = type(value)
157162
if cls not in _AUTO_WRAPPED_CLASSES:
158163
wrapped = type(
159164
cls.__name__,
160165
(cls,),
161166
{"forward": lambda self, *args, **kwargs: self.call(*args, **kwargs)},
162167
)
163168
_AUTO_WRAPPED_CLASSES[cls] = torch_module(wrapped)
164-
wrapped_cls = _AUTO_WRAPPED_CLASSES[cls]
169+
return _AUTO_WRAPPED_CLASSES[cls]
170+
171+
172+
def _auto_wrap_native_op(value: NativeOP) -> torch.nn.Module:
173+
"""Auto-wrap any NativeOP as a torch.nn.Module via ``torch_module``.
174+
175+
Parameters
176+
----------
177+
value : NativeOP
178+
The dpmodel object to wrap.
179+
180+
Returns
181+
-------
182+
torch.nn.Module
183+
The wrapped pt_expt module, deserialized from value's serialized state.
184+
"""
185+
cls = type(value)
186+
wrapped_cls = auto_wrapped_class(cls)
165187
if not (hasattr(value, "serialize") and hasattr(wrapped_cls, "deserialize")):
166188
raise TypeError(
167189
f"Cannot auto-wrap {cls.__name__}: "

deepmd/pt_expt/model/make_model.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
cuda_infer_level,
2525
)
2626
from deepmd.pt_expt.common import (
27+
auto_wrapped_class,
2728
torch_module,
2829
)
2930
from deepmd.pt_expt.utils.graph_builder import (
@@ -422,7 +423,14 @@ def make_model(
422423
The model.
423424
424425
"""
425-
DPModel = make_model_dp(T_AtomicModel)
426+
# Hand the dpmodel CM the WRAPPED atomic class so `self.atomic_model =
427+
# T_AtomicModel(...)` constructs the pt_expt module directly with the
428+
# live (already wrapped) descriptor/fitting. Passing the raw dpmodel
429+
# class instead would build a raw atomic model and convert the instance
430+
# through a serialize()/deserialize() round-trip, which drops
431+
# runtime-only configuration (e.g. the DPA4 `use_amp` switch) that the
432+
# portable record deliberately does not carry.
433+
DPModel = make_model_dp(auto_wrapped_class(T_AtomicModel))
426434

427435
@torch_module
428436
class CM(DPModel, *T_Bases):

source/tests/common/dpmodel/test_descrpt_dpa4.py

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -265,26 +265,18 @@ def test_supported_feature_roundtrip(self, overrides) -> None:
265265
out2 = np.asarray(dd2.call(coord.reshape(nf, -1), atype, nlist)[0])
266266
np.testing.assert_array_equal(out1, out2)
267267

268-
@pytest.mark.parametrize(
269-
"use_amp",
270-
[
271-
True, # the constructor default; must not be clobbered either
272-
False, # the value that was silently lost, re-enabling autocast
273-
],
274-
)
275-
def test_use_amp_survives_roundtrip(self, use_amp) -> None:
276-
"""``use_amp`` must round-trip through serialize/deserialize.
277-
278-
The key was missing from the config, so a backend that rebuilds from
279-
it (pt_expt does) reset ``use_amp: false`` to True and kept training in
280-
bfloat16. The forward-output round-trip test can't catch this --
281-
dpmodel never autocasts, so outputs match either way.
268+
def test_use_amp_stays_out_of_the_portable_record(self) -> None:
269+
"""``use_amp`` is a runtime/training policy, not model state.
270+
271+
The portable serialization must not carry it (a ``use_amp: true``
272+
record would e.g. be rejected by the JAX deserializer); a fresh
273+
deserialize falls back to the constructor default. Construction-time
274+
survival is pinned at the pt_expt assembly boundary instead
275+
(``test_get_model_dpa4.py``).
282276
"""
283-
dd = make_descriptor(use_amp=use_amp)
284-
assert dd.use_amp is use_amp
285-
assert dd.serialize()["config"]["use_amp"] is use_amp
286-
dd2 = DescrptDPA4.deserialize(dd.serialize())
287-
assert dd2.use_amp is use_amp
277+
dd = make_descriptor(use_amp=False)
278+
assert dd.use_amp is False
279+
assert "use_amp" not in dd.serialize()["config"]
288280

289281
def test_legacy_spin_gate_is_squared_on_deserialize(self) -> None:
290282
"""Version 1.2 stores the env-seed spin gate after the quadratic form.

source/tests/pt_expt/model/test_get_model_dpa4.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,3 +285,61 @@ def test_unrelated_construction_error_propagates(self) -> None:
285285

286286
if __name__ == "__main__":
287287
unittest.main()
288+
289+
290+
class TestUseAmpSurvivesAssembly(unittest.TestCase):
291+
"""``use_amp`` is runtime policy: it must survive model ASSEMBLY without
292+
entering the portable serialization record (which the JAX deserializer
293+
rejects for ``use_amp: true``). The wrapping of the atomic model must
294+
therefore not round-trip the constructed descriptor through
295+
``serialize()``/``deserialize()``.
296+
"""
297+
298+
def test_get_model_keeps_use_amp_false(self) -> None:
299+
model = get_model(
300+
_make_raw_model_config(
301+
descriptor={
302+
"sel": 20,
303+
"rcut": 4.0,
304+
"channels": 8,
305+
"n_radial": 4,
306+
"lmax": 1,
307+
"mmax": 1,
308+
"n_blocks": 1,
309+
"precision": "float64",
310+
"seed": 1,
311+
"use_amp": False,
312+
}
313+
)
314+
)
315+
assert model.atomic_model.descriptor.use_amp is False
316+
317+
def test_get_model_keeps_the_use_amp_default(self) -> None:
318+
model = get_model(_make_raw_model_config())
319+
assert model.atomic_model.descriptor.use_amp is True
320+
321+
def test_standard_type_keeps_use_amp_false(self) -> None:
322+
"""The plain `standard` route wraps through the same boundary."""
323+
cfg = _make_raw_model_config(
324+
descriptor={
325+
"type": "dpa4",
326+
"sel": 20,
327+
"rcut": 4.0,
328+
"channels": 8,
329+
"n_radial": 4,
330+
"lmax": 1,
331+
"mmax": 1,
332+
"n_blocks": 1,
333+
"precision": "float64",
334+
"seed": 1,
335+
"use_amp": False,
336+
},
337+
fitting_net={
338+
"type": "dpa4_ener",
339+
"precision": "float64",
340+
"seed": 1,
341+
},
342+
)
343+
del cfg["type"]
344+
model = get_model(cfg)
345+
assert model.atomic_model.descriptor.use_amp is False

0 commit comments

Comments
 (0)