Implement NHS handlers to avoid redundant copies of the same grid NHS#1222
Implement NHS handlers to avoid redundant copies of the same grid NHS#1222efaulhaber wants to merge 12 commits into
Conversation
Co-authored-by: RubberLanding <nico.schumann@tutanota.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1222 +/- ##
==========================================
- Coverage 90.05% 90.02% -0.03%
==========================================
Files 136 136
Lines 10594 10641 +47
==========================================
+ Hits 9540 9580 +40
- Misses 1054 1061 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors how Semidiscretization stores neighborhood searches by introducing an AbstractNHSHandler abstraction with two concrete implementations: PairsNHSHandler (one NHS per ordered system pair, the previous behavior) and SharedNHSHandler (one NHS per neighbor system and required radius, used by default for GridNeighborhoodSearch). The motivation is to avoid storing and updating redundant copies of identical grid-based neighborhood searches when the underlying search supports querying arbitrary points after an update.
Changes:
- Introduce
AbstractNHSHandler,PairsNHSHandler,SharedNHSHandler, anddefault_neighborhood_search_handler; route lookups, GPU adaptation, and display through the handler. - Replace
Semidiscretization.neighborhood_searchesmatrix withneighborhood_search_handler; update all downstream code (interpolation, split integration, GPU transfer, TLSPH path, tests). - Add documentation section for handlers and new tests covering default selection, explicit handler choices, lookup radius semantics, and invalid handler arguments.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/TrixiParticles.jl | Export PairsNHSHandler/SharedNHSHandler and re-export AbstractNeighborhoodSearch. |
| src/general/neighborhood_search.jl | Add handler abstraction, two implementations, default selection, and refactored lookups. |
| src/general/semidiscretization.jl | Replace neighborhood_searches field with neighborhood_search_handler; update constructor, show, and semidiscretize. |
| src/general/gpu.jl | Provide handler-aware GPU/CPU adaptation. |
| src/general/interpolation.jl | Switch from matrix slice to per-system handler lookup. |
| src/callbacks/split_integration.jl | Use get_neighborhood_search instead of direct matrix indexing for periodic box extraction. |
| src/preprocessing/particle_packing/nhs_faces.jl | Use re-exported AbstractNeighborhoodSearch. |
| docs/src/general/neighborhood_search.md | Document handler selection with examples. |
| test/general/semidiscretization.jl | Add tests for default handler selection, explicit handlers, and invalid handler arguments. |
| test/examples/examples_fluid.jl | Add PairsNHSHandler fluid example and update field accesses. |
| test/examples/gpu.jl | Add PairsNHSHandler GPU variant and update GPU field accesses. |
| test/callbacks/mechanical_work_calculator.jl | Use PairsNHSHandler (no grid NHS available for the mock system). |
| test/count_allocations.jl | Wrap handler internals with NoUpdateNeighborhoodSearch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Extracted from #1088.
This is the first part of improving NHS efficiency. It builds the handler structure and avoids storing multiple identical neighborhood searches without touching the NHS update process. This means instead of updating multiple redundant searches, we now update one search multiple times. The next PR avoids the redundant updates.