Skip to content

feat(pt/ptexpt): Filter LAMMPS neighbor lists by model cutoff#5526

Closed
OutisLi wants to merge 1 commit into
deepmodeling:masterfrom
OutisLi:pr/cppnlist
Closed

feat(pt/ptexpt): Filter LAMMPS neighbor lists by model cutoff#5526
OutisLi wants to merge 1 commit into
deepmodeling:masterfrom
OutisLi:pr/cppnlist

Conversation

@OutisLi

@OutisLi OutisLi commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Trim LAMMPS-provided neighbor lists to the model cutoff before building PyTorch/PTExpt lower-interface tensors.
  • Share the cutoff filtering logic through NeighborListData and apply it to PT, PTExpt, spin, and tensor C++ inference paths.
  • Add regression coverage for skin-list neighbors that stay within the model nlist width but lie outside rcut.

Test plan

  • ninja -C /tmp/deepmd-kit-api-cc-test runUnitTests_cc
  • runUnitTests_cc --gtest_filter='TestNeighborListData.FilterByDistance*:TestInferDeepPotAPt/*cpu_lmp_nlist_skin_below_model_width*'

Summary by CodeRabbit

  • New Features

    • Added neighbor list distance-based filtering that removes neighbors beyond a specified cutoff distance before tensor padding in model computation.
  • Tests

    • Added tests verifying neighbor list filtering behavior with LAMMPS-style neighbor list inputs across multiple model types.

@coderabbitai

coderabbitai Bot commented Jun 13, 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: a2c83e6c-742c-4380-b345-3ba924dcb09d

📥 Commits

Reviewing files that changed from the base of the PR and between 5d94bd6 and a20a6bd.

📒 Files selected for processing (10)
  • source/api_cc/include/common.h
  • source/api_cc/src/DeepPotPT.cc
  • source/api_cc/src/DeepPotPTExpt.cc
  • source/api_cc/src/DeepSpinPT.cc
  • source/api_cc/src/DeepSpinPTExpt.cc
  • source/api_cc/src/DeepTensorPT.cc
  • source/api_cc/src/common.cc
  • source/api_cc/tests/test_deeppot_pt.cc
  • source/api_cc/tests/test_deeppot_ptexpt.cc
  • source/api_cc/tests/test_neighbor_list_data.cc

📝 Walkthrough

Walkthrough

The PR adds a filter_by_distance method to trim neighbor lists by distance cutoff across DeepMD's C++ inference API. The method is implemented, integrated into five compute classes, and validated through unit and integration tests.

Changes

Neighbor List Distance Filtering

Layer / File(s) Summary
Filter method implementation and unit tests
source/api_cc/include/common.h, source/api_cc/src/common.cc, source/api_cc/tests/test_neighbor_list_data.cc
New templated filter_by_distance method on NeighborListData filters each neighbor-list row in-place by comparing squared inter-particle distances against a cutoff. Rows with invalid center indices are cleared. Implementation includes explicit instantiations for double and float, and two unit tests verify distance computation using center atom indices and handling of invalid rows.
Integration into compute methods
source/api_cc/src/DeepPotPT.cc, source/api_cc/src/DeepPotPTExpt.cc, source/api_cc/src/DeepSpinPT.cc, source/api_cc/src/DeepSpinPTExpt.cc, source/api_cc/src/DeepTensorPT.cc
Five compute implementations now call filter_by_distance on nlist_data during neighbor-list rebuilding (ago == 0), using current coordinates and model cutoff, immediately before padding.
Integration test coverage
source/api_cc/tests/test_deeppot_pt.cc, source/api_cc/tests/test_deeppot_ptexpt.cc
Two typed tests (cpu_lmp_nlist_skin_below_model_width) verify inference consistency when neighbor lists contain skin neighbors beyond the model cutoff. Tests construct filtered neighbor lists, run compute, fold forces back, and assert energy, forces, and virial match reference values within tolerance.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

C++, LAMMPS

Suggested reviewers

  • iProzd
  • njzjz-bot
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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: filtering LAMMPS neighbor lists by model cutoff across PT/PTExpt inference paths.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 and usage tips.

@codecov

codecov Bot commented Jun 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.24051% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.20%. Comparing base (5d94bd6) to head (a20a6bd).

Files with missing lines Patch % Lines
source/api_cc/tests/test_deeppot_pt.cc 85.93% 9 Missing ⚠️
source/api_cc/tests/test_deeppot_ptexpt.cc 88.67% 6 Missing ⚠️
source/api_cc/src/common.cc 91.66% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #5526    +/-   ##
========================================
  Coverage   82.19%   82.20%            
========================================
  Files         891      891            
  Lines      101599   101757   +158     
  Branches     4242     4259    +17     
========================================
+ Hits        83507    83647   +140     
- Misses      16789    16804    +15     
- Partials     1303     1306     +3     

☔ 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.

@OutisLi OutisLi closed this Jun 13, 2026
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.

1 participant