Skip to content

Make BackendModelFactory class injection complete and required (no silent dpmodel fallback) #5977

Description

@wanghan-iapcm

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

  1. 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).
  2. Remove the fallback imports (LinearEnergyAtomicModel as LinearEnergyAtomicModelDP, linear_atomic_model or ...) from the factory body.
  3. 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 unused inner_potential_model parameter 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 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.

References

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions