deepmodeling / deepmodeling/deepmd-kit

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

Open
#5,977 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Python
Stars
2k
Forks
649
Avg merge
6d 18h
Merged PRs (30d)
15

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

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 #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 #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

- #5964 — introduced the shared factory / canonical `linear_ener` composition.
- #5960 — the `use_amp` silent-loss bug caused by the hardcoded linear class; fixed by injecting `linear_atomic_model` (pt_expt only).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.