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

Đang mở
#5,941 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 3 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức phù hợp với người mới
48/100
Loại issue
Lỗi
Độ rõ ràng
Khá rõ ràng
Mức độ hoạt động
Ít trao đổi
Lĩnh vực
backend, testing

Hướng nghiên cứu

Bắt đầu với DeepEvalBackend.init và DeepEval.init trong deepmd/infer/deep_eval.py, DeepPot trong deepmd/infer/deep_pot.py và TestDeepPotNeighborList trong source/tests/infer/test_models.py. So sánh cách xử lý neighbor_list giữa các phần mở rộng backend đã được lập tài liệu, sau đó thiết lập và lập tài liệu cho một contract nhất quán. Công việc được xem là hoàn tất khi các docstring dùng chung và các test bao quát rõ ràng mọi phần mở rộng, bao gồm cả các mục ma trận trong tương lai.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

bug

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.

Ngôn ngữ chính
Python
Star
2.1k
Fork
663
Merge trung bình
3 ngày 11 giờ
Pull request đã merge (30 ngày)
25

Chuẩn bị môi trường

Mở trong Codespaces

Khởi chạy dev container của dự án ngay trên trình duyệt, bằng tài khoản GitHub của bạn.

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của deepmodeling/deepmd-kit

Tất cả issue của deepmodeling/deepmd-kit

Issue tương tự

Thêm issue về Python

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.