Skip to content

[BUG] neighbor_list behaves three different ways across backends (implemented / silently ignored / raising) #5941

Description

@wanghan-iapcm

neighbor_list is a documented keyword on DeepPot / DeepEval, but its behaviour depends on the model file extension in a way nothing surfaces to the caller. After #5858 there are three distinct outcomes:

Backend / extension Behaviour
.pb (tf), .pt2 (pt_expt) Implemented — the ASE object is used to build the neighbor list
.pth (pt), .pd, .dp, .savedmodel (jax) Accepted and silently ignored
.savedmodeltf (tf2) Raises NotImplementedError (added by #5858)

Note deepmd/pt/infer/deep_eval.py belongs in the silently-ignored group despite storing the value: its only read is ase_provided = self.neighbor_list is not None, so the ASE object itself never builds a neighbor list. pd, jax and dpmodel additionally document the parameter while ignoring it.

The silent-no-op cases are the harmful ones — a user passing a custom neighbor list gets the model's built-in one and results that look plausible.

Two concrete gaps

Docstrings promise it unconditionally. DeepEvalBackend.__init__ and DeepEval.__init__ in deepmd/infer/deep_eval.py, and DeepPot in deepmd/infer/deep_pot.py, all say:

The ASE neighbor list class to produce the neighbor list. If None, the neighbor list will be built natively in the model.

So DeepPot("model.savedmodeltf", neighbor_list=nl) is a documented call that now raises, and DeepPot("model.pth", neighbor_list=nl) is a documented call that quietly ignores the argument.

No test would catch the next backend. TestDeepPotNeighborList in source/tests/infer/test_models.py is parameterized to (".pb",) while the base TestDeepPot runs a four-way extension matrix:

https://github.com/deepmodeling/deepmd-kit/blob/69ff61ef1eb3a3ad1a3f42f10d1e5cd1f8d0f0ba/source/tests/infer/test_models.py#L399-L403

A tf2-only "it raises" test cannot prevent recurrence; extending this matrix would.

Suggested direction

Decide the contract once, rather than per backend: either every backend that cannot honour neighbor_list raises NotImplementedError as tf2 now does, or the parameter is documented as advisory. Whichever is chosen, the shared docstrings should state it, and TestDeepPotNeighborList should cover every extension so a new backend inherits the expectation.

Raised while reviewing #5858, which deliberately scoped itself to the tf2 half.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions