Skip to content

fix(dpmodel): exclude virtual atoms from neighbor stats - #5834

Open
njzjz-bot wants to merge 1 commit into
deepmodeling:masterfrom
njzjz-bot:fix/dpmodel-neighbor-stat-virtual-atoms-5630
Open

fix(dpmodel): exclude virtual atoms from neighbor stats#5834
njzjz-bot wants to merge 1 commit into
deepmodeling:masterfrom
njzjz-bot:fix/dpmodel-neighbor-stat-virtual-atoms-5630

Conversation

@njzjz-bot

@njzjz-bot njzjz-bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • require both center and neighbor atoms to be real before distance and neighbor-count reductions
  • preserve periodic self-image neighbors while excluding only the original self pair
  • use numeric count masks for strict Array API reductions
  • add periodic/non-periodic and mixed/non-mixed virtual-atom regression coverage

Why existing tests missed this

The existing neighbor-stat tests build datasets containing only type-0 real atoms. They validate aggregate CLI statistics across cutoffs and backends, so neither a negative center type nor a negative neighbor type ever reaches NeighborStatOP. The new geometry contains an overlapping virtual neighbor that previously forced the minimum distance to zero and a virtual center that previously inflated the type-0 maximum neighbor count from one to two.

Validation

  • ruff format .
  • ruff check .
  • focused NumPy and strict Array API virtual-atom tests
  • existing NumPy, PyTorch, and JAX neighbor-stat suites, including periodic self-image behavior
  • TensorFlow C++ core regression test
  • total focused result: 6 passed with 30 subtests
  • manual NumPy, Array API Strict, PyTorch, and JAX virtual-atom scenario

Closes #5630

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

Summary by CodeRabbit

  • Bug Fixes

    • Virtual atoms and invalid neighbor pairs are now excluded from distance and neighbor-count calculations.
    • Neighbor statistics now correctly handle self-pairs and virtual entries across mixed and non-mixed type modes.
    • Invalid distances are safely ignored when determining minimum neighbor distances.
  • Tests

    • Added coverage for virtual-atom masking, distance calculations, neighbor counts, output shapes, and array metadata preservation.

Mask virtual centers and neighbors before distance and count reductions. Add periodic, mixed-type, and strict Array API regression coverage.

Coding-Agent: Codex
Codex-Version: codex-cli 0.144.4
Model: gpt-5.6-sol
Reasoning-Effort: xhigh
@dosubot dosubot Bot added the bug label Jul 16, 2026
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 65da5272-1af9-4dd6-bc52-23e763700259

📥 Commits

Reviewing files that changed from the base of the PR and between 6c3b985 and 7ddcaae.

📒 Files selected for processing (3)
  • deepmd/dpmodel/utils/neighbor_stat.py
  • source/tests/common/dpmodel/array_api/test_neighbor_stat.py
  • source/tests/common/dpmodel/test_neighbor_stat.py

📝 Walkthrough

Walkthrough

NeighborStatOP now masks self-pairs and virtual atoms before distance and neighbor-count reductions. New tests cover mixed and per-type modes, periodic and non-periodic cells, and Array API output consistency.

Changes

Virtual atom neighbor statistics

Layer / File(s) Summary
Valid-pair masking and reductions
deepmd/dpmodel/utils/neighbor_stat.py
Distance and neighbor-count calculations use a mask excluding self-pairs and atoms with negative types.
Neighbor statistics regression coverage
source/tests/common/dpmodel/test_neighbor_stat.py, source/tests/common/dpmodel/array_api/test_neighbor_stat.py
Tests verify virtual atoms are excluded across cell configurations, counting modes, and Array API properties.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

Suggested reviewers: iprozd

🚥 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 clearly and concisely describes the main change: excluding virtual atoms from neighbor statistics.
Linked Issues check ✅ Passed The fix masks virtual centers and neighbors before distance and count reductions and adds mixed/non-mixed regression tests for virtual atoms.
Out of Scope Changes check ✅ Passed The changes stay focused on neighbor-stat masking and regression tests, with no unrelated scope visible.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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

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

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.79%. Comparing base (6c3b985) to head (7ddcaae).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5834      +/-   ##
==========================================
- Coverage   78.58%   77.79%   -0.80%     
==========================================
  Files        1050     1050              
  Lines      120637   120638       +1     
  Branches     4356     4356              
==========================================
- Hits        94801    93845     -956     
- Misses      24278    25226     +948     
- Partials     1558     1567       +9     

☔ 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

njzjz commented Jul 18, 2026

Copy link
Copy Markdown
Member

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

  • @wanghan-iapcm — 1 commits on changed files (deepmd/dpmodel/utils/neighbor_stat.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

@njzjz
njzjz requested review from wanghan-iapcm and removed request for wanghan-iapcm July 18, 2026 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Code scan] Ignore virtual atoms in dpmodel neighbor statistics

2 participants