fix(tf): preserve non-PBC ASE neighbor semantics

[njzjz-bot]

Closes #5668.

Summary

• preserve the original cells=None decision separately from the identity box required by TensorFlow placeholders
• pass a missing cell to ASE for open systems and make both TF1 neighbor-list builders handle that documented input without creating periodic ghosts
• correct the builders return annotations and document their open-boundary behavior
• add collected regressions for DeepPotential and for DeepTensor eval/eval_full

Why existing tests missed this

• the original external ASE neighbor-list tests were designed around periodic systems and never exercised cells=None
• the parameterized TestDeepPot symbol is replaced by object, so TestDeepPotNeighborList did not inherit the base evaluation tests; collection contained only two locally declared skipped tests
• although the DeepPotential testcase YAML contains a box: null result, it therefore never ran through the ASE-backed evaluator
• DeepTensor neighbor-list tests inherited PBC fixtures and had no native-versus-ASE open-boundary comparison

Validation

• focused DeepPotential no-PBC regression: passed
• focused DeepTensor eval/eval_full no-PBC regression: passed
• full TestDeepDipoleNewPBCNeighborList class: 10 passed, 2 skipped
• TF1 DeepPotential periodic native-versus-ASE parity smoke: passed
• pytest collect-only confirms both new regressions are collected
• ruff format .
• ruff check .
• git diff --check

Coding agent: Codex
Codex version: codex-cli 0.144.4
Model: gpt-5.6-sol
Reasoning effort: xhigh

Summary by CodeRabbit

• Bug Fixes

  • Fixed neighbor-list construction for open-boundary (no PBC) systems to preserve correct boundary semantics.
  • Ensured consistent neighbor-list handling across TensorFlow inference paths (including energy/tensor and dipole evaluation), so results no longer depend on placeholder cell inputs.

• Tests

  • Added new no-PBC neighbor-list verification against reference outputs (forces, energies, and virials).
  • Added dipole coverage confirming neighbor-list-based results match native neighbor-list behavior, including legacy implementation checks.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dc56bfae-eb7c-4bc9-8a68-cc08d2034a72

📥 Commits

Reviewing files that changed from the base of the PR and between f75ab0b and 9dd4210.

📒 Files selected for processing (3)

• deepmd/tf/infer/deep_eval.py
• source/tests/infer/test_models.py
• source/tests/tf/test_deepdipole.py

🚧 Files skipped from review as they are similar to previous changes (1)

• deepmd/tf/infer/deep_eval.py

---

📝 Walkthrough

Walkthrough

The change preserves open-boundary semantics for TensorFlow inference with ASE neighbor lists, separates placeholder cells from boundary cells, updates neighbor-list annotations and documentation, and adds regression tests for DeepEval and DeepDipole.

Changes

Non-PBC neighbor-list inference

Layer / File(s)	Summary
DeepEval boundary handling deepmd/tf/infer/deep_eval.py	DeepEval and DeepEvalOld accept None as an open-boundary cell, use zero cells for neighbor-list construction, preserve periodicity separately, and update return annotations.
DeepTensor boundary handling deepmd/tf/infer/deep_tensor.py	eval and eval_full pass cells to neighbor-list construction only when periodic boundaries are enabled.
Open-boundary regression coverage source/tests/infer/test_models.py, source/tests/tf/test_deepdipole.py	Tests validate non-PBC ASE neighbor-list outputs against reference values and native DeepDipole evaluation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant DeepEval
  participant ASENeighborList
  participant TensorFlow
  Caller->>DeepEval: Evaluate with box=None
  DeepEval->>ASENeighborList: Build neighbors with neighbor_cell=None
  ASENeighborList-->>DeepEval: Return non-periodic neighbor data
  DeepEval->>TensorFlow: Feed placeholder cell separately
  TensorFlow-->>Caller: Return inference results

Loading

Suggested labels: Docs

Suggested reviewers: wanghan-iapcm

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the main fix for non-PBC ASE neighbor semantics.
Linked Issues check	✅ Passed	The PR preserves non-PBC state, updates TF1 neighbor-list handling, and adds regression tests for DeepEval and DeepTensor as requested.
Out of Scope Changes check	✅ Passed	The changes stay focused on non-PBC neighbor-list handling, documentation, and related regression tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.89%. Comparing base (6c3b985) to head (9dd4210).
⚠️ Report is 39 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5859      +/-   ##
==========================================
+ Coverage   78.58%   78.89%   +0.31%     
==========================================
  Files        1050     1067      +17     
  Lines      120637   123808    +3171     
  Branches     4356     4515     +159     
==========================================
+ Hits        94801    97681    +2880     
- Misses      24278    24518     +240     
- Partials     1558     1609      +51     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[njzjz]

Possible reviewers based on changed lines, exact file history, and exact-file review history:

• @wanghan-iapcm — 4 commits on changed files; 17 reviews on exact changed files (deepmd/tf/infer/deep_eval.py, source/tests/infer/test_models.py).
• @iProzd — 2 commits on changed files (deepmd/tf/infer/deep_eval.py, source/tests/infer/test_models.py).

No review request was made automatically.

Coding agent: Codex
Codex version: codex-cli 0.144.4
Model: gpt-5.6-sol
Reasoning effort: xhigh

[wanghan-iapcm]

The diagnosis and the fix are both right. _prepare_feed_dict overwrote cells with a 1 Angstrom identity box to satisfy the TF box placeholder, after which the call site's cells if cells is not None else None could never be None, so build_neighbor_list re-derived pbc from cell is not None and ASE generated periodic images of a 1 Angstrom cell for a system with no cell. Threading the real boundary condition through a separate neighbor_cell is the correct separation, and substituting a zero 3x3 only where ASE needs an array - after pbc has already been computed - matches what deepmd/pt_expt/infer/deep_eval.py has done since #5284, so this converges TF with the existing reference rather than adding a third convention.

I checked the parts most likely to break rather than assuming them: the periodic path is a literal identity (neighbor_cell = cells if pbc else None reduces to the old expression when pbc is true); multi-frame is guarded by NotImplementedError at all three call sites, so no [nframes, 9] array reaches reshape(3, 3); a zero cell is safe for ASE's NewPrimitiveNeighborList with pbc=False (no ghosts, no divide-by-zero, fine down to one atom); the empty-ghost bookkeeping degenerates to a no-op; and the corrected 6-tuple return annotation matches what both copies of build_neighbor_list actually return.

The regression test is the strongest part. Run against the base commit it fails with 18/18 force components wrong, max absolute difference 3.54 and max relative difference around 4800x - exactly the signature of the spurious ghosts - and passes at the head. That is a real bug with a real test, not a test that merely accompanies a fix.

Two non-blocking notes:

1. The deepmd/tf/infer/deep_tensor.py hunks are not exercised by anything in this PR. source/tests/tf/test_deepdipole.py imports DeepDipole from deepmd.tf.infer, which re-exports the generic deepmd.infer.deep_dipole.DeepDipole; its MRO runs through deepmd.infer.deep_tensor.DeepTensor and into deepmd/tf/infer/deep_eval.py, never through the TF-specific deep_tensor.py. Patching only deep_tensor.py leaves that test failing (max difference 1.34); patching only deep_eval.py makes it pass. The change itself is correct and mirrors the deep_eval.py fix, but the PR description's "focused DeepTensor eval/eval_full no-PBC regression: passed" is attributed to the wrong file - worth correcting so the claim is not taken at face value later. The only subclass of the TF DeepTensor is DeepDipoleOld, whose sole consumer never passes a neighbor_list, so the code is currently unreachable rather than wrong.

2. The comment correctly identifies why TestDeepPotNeighborList inherits nothing - parameterized() rebinds the decorated name to object - and works around it with a standalone function. That is a fair call for this PR, but the underlying breakage dates to #4028 and means the class has contributed zero assertions since 2024-08-04, which is why this bug survived. Worth a follow-up to repair the class rather than leaving the next contributor to rediscover it.

Also minor: the substituted zero cell uses dtype=coords.dtype, so integer coordinates would yield an integer cell. It has no observable effect here (the branch only runs with pbc=False, where the offsets are all zero, and ASE upcasts internally), but the pt_expt equivalent hardcodes np.float64 and it would be nice to match.

[njzjz]

Addressed the review follow-ups in commit 9dd4210:

• Restored the parameterized ASE neighbor-list test inheritance, so the class now contributes its full assertion suite.
• Extended the no-PBC dipole regression to cover both the public TensorFlow evaluator and the legacy TF DeepTensor implementation.
• Changed the open-boundary fallback cells to float64, matching the existing reference implementation.

Validation:

• ruff format .
• ruff check .
• Focused affected tests: 19 passed, 5 skipped

Coding agent: Codex
Codex version: codex-cli 0.144.6
Model: gpt-5.6-sol
Reasoning effort: xhigh