Repository navigation
Make BackendModelFactory class injection complete and required (no silent dpmodel fallback) #5977
Description
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 the use_amp loss fixed in #5960 and should be closed off structurally.
Problem
Current BackendModelFactory.__init__:
- required:
descriptor_base,fitting_base,model_base,backend_name - optional (default
None→ dpmodel class):atomic_model,pairtab_model,zbl_model,linear_atomic_model - 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
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).
- Remove the fallback imports (
LinearEnergyAtomicModel as LinearEnergyAtomicModelDP,linear_atomic_model or ...) from the factory body. - Open question —
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:- (a) inject it like the others, for uniformity; or
- (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 raisesTypeErrorat 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 LinearEnergyAtomicModelDPpattern). - The
InnerPotentialAtomicModeldecision (3a or 3b) is implemented and pinned by a test.
References
- refactor(model): express analytical bridging as an explicit linear_ener composition #5964 — introduced the shared factory / canonical
linear_enercomposition. - perf(dpa4): batch the SO3/grid contractions over (D,F); keep use_amp through pt_expt assembly #5960 — the
use_ampsilent-loss bug caused by the hardcoded linear class; fixed by injectinglinear_atomic_model(pt_expt only).
Activity