Skip to content

Add NeighborSearch AdjacentAtomInfo compatibility adapter#7359

Closed
PKUJZX wants to merge 1 commit into
deepmodeling:developfrom
PKUJZX:pr7358-neighlist-adapter
Closed

Add NeighborSearch AdjacentAtomInfo compatibility adapter#7359
PKUJZX wants to merge 1 commit into
deepmodeling:developfrom
PKUJZX:pr7358-neighlist-adapter

Conversation

@PKUJZX
Copy link
Copy Markdown

@PKUJZX PKUJZX commented May 18, 2026

Summary

Part of #7358.

This PR adds the first compatibility layer needed before module_neighlist can be considered as a replacement candidate for the existing module_neighbor path.

The change is intentionally small and non-invasive:

  • It does not switch any production caller to the new backend.
  • It does not modify Grid_Driver::Find_atom, atom_arrange::search, or ESolver_LJ.
  • It only adds the metadata and adapter needed to compare NeighborSearch output against the existing AdjacentAtomInfo contract.

What changed

  • Preserve periodic image offsets in NeighborAtom as cell_x/cell_y/cell_z.
  • Populate those offsets in NeighborSearch::setMemberVariables.
  • Add convert_neighbor_search_to_adjs(...), which converts NeighborSearch results into AdjacentAtomInfo.
  • Preserve the historical AdjacentAtomInfo behavior:
    • adj_num counts only non-self neighbors.
    • The center atom is appended as the last entry.
    • The center atom uses box = (0, 0, 0).

What this PR deliberately does not do

This PR does not attempt a backend switch or broader refactor. In particular, it does not:

  • replace the current Grid_Driver implementation,
  • change LCAO/DeepKS/RT/operator call paths,
  • add MPI domain decomposition,
  • change LJ behavior,
  • address allocator sizing or performance benchmarking.

Those should remain separate follow-up PRs after this adapter is proven.

Test coverage

I added old-vs-new equivalence tests that compare the adapter output against the existing Grid_Driver implementation.

Covered scenarios:

  • Existing Si fixture with normal PBC neighbor search.
  • Orthogonal two-atom boundary case, explicitly checking periodic image boxes such as (-1, 0, 0) and (1, 0, 0).
  • Skewed-cell case to verify image metadata and coordinates remain consistent with the old implementation.
  • Direct NeighborSearch unit coverage proving generated periodic images retain cell_x/cell_y/cell_z.

The tests compare neighbor identity using (type, atom index, box) and compare coordinates with floating-point tolerance. They also assert the self-last contract.

Verification

Environment dependencies were installed and the planned CMake/CTest flow was run successfully:

cmake -B build-neighlist-pr1 -DBUILD_TESTING=ON
cmake --build build-neighlist-pr1 --target MODULE_CELL_NEIGHBOR_neighbor_search MODULE_CELL_NEIGHBOR_neighlist_adapter -j2
ctest --test-dir build-neighlist-pr1 -R "MODULE_CELL_NEIGHBOR_(neighbor_search|neighlist_adapter)" --output-on-failure

@mohanchen mohanchen requested a review from 19hello May 18, 2026 12:18
@mohanchen
Copy link
Copy Markdown
Collaborator

Nice try. I suggest not rushing to apply the nearest neighbor search algorithm to LCAO for now. Since LCAO covers a wide range of applications, potential bugs may easily occur. It can be firstly adopted in empirical potential functions and machine learning potential function models.

@mohanchen mohanchen added Refactor Refactor ABACUS codes Feature Discussed The features will be discussed first but will not be implemented soon labels May 18, 2026
@PKUJZX PKUJZX closed this May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature Discussed The features will be discussed first but will not be implemented soon Refactor Refactor ABACUS codes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants