Skip to content

Implement option to transpose neighbor lists of PrecomputedNeighborhoodSearch#137

Merged
svchb merged 6 commits intomainfrom
ef/transposed-neighbor-list
Dec 2, 2025
Merged

Implement option to transpose neighbor lists of PrecomputedNeighborhoodSearch#137
svchb merged 6 commits intomainfrom
ef/transposed-neighbor-list

Conversation

@efaulhaber
Copy link
Member

@efaulhaber efaulhaber commented Nov 28, 2025

This PR implements the "transposed NL" optimization mentioned in trixi-framework/TrixiParticles.jl#968. See this PR for benchmark results.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements an option to transpose the neighbor lists data structure in PrecomputedNeighborhoodSearch to optimize GPU performance through coalesced memory access patterns. When transpose_backend = true, the first neighbors of all points are stored contiguously in memory rather than all neighbors of each point, which can provide up to 3x speedup on GPUs.

  • Added transpose_backend parameter to PrecomputedNeighborhoodSearch and related functions
  • Implemented transposed memory layout using PermutedDimsArray wrapper in DynamicVectorOfVectors
  • Added comprehensive tests to verify both regular and transposed backend behavior
  • Updated documentation to explain when to use transposed backend (GPU vs CPU optimization)

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/unittest.jl Added inclusion of new test file for precomputed neighborhood search
test/nhs_precomputed.jl New test file verifying both regular and transposed backend memory layouts
src/vector_of_vectors.jl Extended DynamicVectorOfVectors constructor to support transpose option and added helper functions to check backend type
src/nhs_precomputed.jl Added transpose_backend parameter with detailed documentation about CPU vs GPU optimization trade-offs
src/nhs_grid.jl Added documentation note about search_radius type determining distance computation type
src/nhs_trivial.jl Added documentation note about search_radius type determining distance computation type
src/cell_lists/cell_lists.jl Updated construct_backend functions to accept and propagate transpose_backend parameter with validation
src/PointNeighbors.jl Exported DynamicVectorOfVectors for public API access

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Nov 28, 2025

Codecov Report

❌ Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.82%. Comparing base (d8b0de2) to head (14bf102).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/cell_lists/cell_lists.jl 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #137      +/-   ##
==========================================
+ Coverage   84.75%   84.82%   +0.07%     
==========================================
  Files          15       15              
  Lines         715      725      +10     
==========================================
+ Hits          606      615       +9     
- Misses        109      110       +1     
Flag Coverage Δ
unit 84.82% <93.75%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

@efaulhaber efaulhaber marked this pull request as ready for review December 1, 2025 12:08
@efaulhaber efaulhaber requested review from LasNikas and svchb December 1, 2025 12:08
LasNikas
LasNikas previously approved these changes Dec 2, 2025
@svchb svchb merged commit 9f86e3e into main Dec 2, 2025
12 checks passed
@svchb svchb deleted the ef/transposed-neighbor-list branch December 2, 2025 11:02
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.

3 participants