You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
BackendModelFactory (deepmd/dpmodel/model/model_factory.py, introduced in #5964) constructs backend model objects from injected backend classes. The injection list is incomplete and partially optional, which allows a wrapping backend to silently fall back to constructing dpmodel-class objects. This is the root cause of the use_amp loss fixed in #5960 and should be closed off structurally.
not injectable at all (hardcoded import inside the factory body): InnerPotentialAtomicModel
For a backend that wraps dpmodel classes (pt_expt), an omitted injection is not an error: the factory builds a dpmodel-class composition, and the backend's model wrapper later converts it via deserialize(serialize()). That conversion keeps only the portable record, so any runtime state the live children hold is silently dropped. This is exactly how #5960's bug happened: the factory hardcoded the dpmodel LinearEnergyAtomicModel, and a configured use_amp: false was lost during pt_expt assembly — numerically invisible to every cross-backend parity test, because the loss changes precision policy, not the math.
A half-injected factory is worse than no sharing: it looks canonical while constructing wrong-backend objects, and the failure mode is silent degradation instead of an error.
Proposal
Make all class-injection parameters required (no None default, no fallback import inside the factory):
atomic_model, pairtab_model, zbl_model — already passed explicitly by all four instantiation sites (deepmd/dpmodel/model/model.py, deepmd/jax/model/model.py, deepmd/tf2/model/model.py, deepmd/pt_expt/model/get_model.py), so this is zero-cost.
linear_atomic_model — currently passed only by pt_expt. dpmodel/jax/tf2 must state their choice explicitly (for them the dpmodel class is correct — the point is that "I use the native class" becomes a visible, reviewed decision instead of a silent default).
Remove the fallback imports (LinearEnergyAtomicModel as LinearEnergyAtomicModelDP, linear_atomic_model or ...) from the factory body.
(b) keep it hardcoded and pin the exemption with a comment + a negative test asserting the class carries no non-portable state.
Acceptance
BackendModelFactory(...) with a missing class raises TypeError at import time of the backend module — a missed injection is a hard failure, not a silent fallback.
No dpmodel-class fallback import remains inside any factory construction path (grep-clean for the or LinearEnergyAtomicModelDP pattern).
The InnerPotentialAtomicModel decision (3a or 3b) is implemented and pinned by a test.
Summary
BackendModelFactory(deepmd/dpmodel/model/model_factory.py, introduced in #5964) constructs backend model objects from injected backend classes. The injection list is incomplete and partially optional, which allows a wrapping backend to silently fall back to constructing dpmodel-class objects. This is the root cause of theuse_amploss fixed in #5960 and should be closed off structurally.Problem
Current
BackendModelFactory.__init__:descriptor_base,fitting_base,model_base,backend_nameNone→ dpmodel class):atomic_model,pairtab_model,zbl_model,linear_atomic_modelInnerPotentialAtomicModelFor a backend that wraps dpmodel classes (pt_expt), an omitted injection is not an error: the factory builds a dpmodel-class composition, and the backend's model wrapper later converts it via
deserialize(serialize()). That conversion keeps only the portable record, so any runtime state the live children hold is silently dropped. This is exactly how #5960's bug happened: the factory hardcoded the dpmodelLinearEnergyAtomicModel, and a configureduse_amp: falsewas lost during pt_expt assembly — numerically invisible to every cross-backend parity test, because the loss changes precision policy, not the math.A half-injected factory is worse than no sharing: it looks canonical while constructing wrong-backend objects, and the failure mode is silent degradation instead of an error.
Proposal
Nonedefault, no fallback import inside the factory):atomic_model,pairtab_model,zbl_model— already passed explicitly by all four instantiation sites (deepmd/dpmodel/model/model.py,deepmd/jax/model/model.py,deepmd/tf2/model/model.py,deepmd/pt_expt/model/get_model.py), so this is zero-cost.linear_atomic_model— currently passed only by pt_expt. dpmodel/jax/tf2 must state their choice explicitly (for them the dpmodel class is correct — the point is that "I use the native class" becomes a visible, reviewed decision instead of a silent default).LinearEnergyAtomicModel as LinearEnergyAtomicModelDP,linear_atomic_model or ...) from the factory body.InnerPotentialAtomicModel: it is constructed inside the factory (model_factory.py:325) from a hardcoded import. Today this is harmless because the model is purely analytical (no trainable arrays, no runtime state), so the wrapping backend's conversion is lossless (verified in perf(dpa4): batch the SO3/grid contractions over (D,F); keep use_amp through pt_expt assembly #5960, where an unusedinner_potential_modelparameter was removed for exactly that reason). But "the conversion happens to be lossless today" was also the implicit assumption for the linear class before perf(dpa4): batch the SO3/grid contractions over (D,F); keep use_amp through pt_expt assembly #5960. Either:Acceptance
BackendModelFactory(...)with a missing class raisesTypeErrorat import time of the backend module — a missed injection is a hard failure, not a silent fallback.or LinearEnergyAtomicModelDPpattern).InnerPotentialAtomicModeldecision (3a or 3b) is implemented and pinned by a test.References
linear_enercomposition.use_ampsilent-loss bug caused by the hardcoded linear class; fixed by injectinglinear_atomic_model(pt_expt only).