Skip to content

Add unsafe versions of foreach_neighbor and foreach_point_neighbor#154

Merged
LasNikas merged 6 commits into
mainfrom
ef/foreach-neighbor-unsafe
Apr 14, 2026
Merged

Add unsafe versions of foreach_neighbor and foreach_point_neighbor#154
LasNikas merged 6 commits into
mainfrom
ef/foreach-neighbor-unsafe

Conversation

@efaulhaber
Copy link
Copy Markdown
Member

@efaulhaber efaulhaber commented Apr 13, 2026

Add unsafe versions of foreach_neighbor and foreach_point_neighbor that skip all bounds checks.

Once again citing @sloede:

We like "unsafe" if it implies raw speed 😈

Copy link
Copy Markdown
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 introduces “unsafe” neighbor-iteration APIs intended to skip bounds checks for performance-critical CPU/GPU loops, and restructures the internal neighbor-loop dispatch to separate a safe function barrier from an inbounds-propagating inner loop.

Changes:

  • Add exported foreach_neighbor_unsafe and foreach_point_neighbor_unsafe APIs.
  • Introduce a foreach_neighbor_inner function-barrier pattern and update GridNeighborhoodSearch to specialize the inner loop.
  • Update documentation and exports to expose the new unsafe entry points.

Reviewed changes

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

File Description
src/neighborhood_search.jl Adds foreach_point_neighbor_unsafe and foreach_neighbor_unsafe, plus a new foreach_neighbor_inner dispatch layer and updated docs.
src/nhs_grid.jl Renames the grid specialization to foreach_neighbor_inner and adjusts comments around bounds-check safety.
src/nhs_precomputed.jl Attempts to adjust the precomputed specialization for the new unsafe/safe split (currently miswired).
src/PointNeighbors.jl Exports the newly added unsafe APIs.

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

Comment thread src/neighborhood_search.jl Outdated
Comment thread src/neighborhood_search.jl Outdated
Comment thread src/nhs_precomputed.jl Outdated
Comment thread src/neighborhood_search.jl Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 14, 2026

Codecov Report

❌ Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.06%. Comparing base (41523f7) to head (8ba18db).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/neighborhood_search.jl 94.11% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #154      +/-   ##
==========================================
+ Coverage   84.82%   86.06%   +1.23%     
==========================================
  Files          15       15              
  Lines         725      739      +14     
==========================================
+ Hits          615      636      +21     
+ Misses        110      103       -7     
Flag Coverage Δ
unit 86.06% <96.66%> (+1.23%) ⬆️

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 April 14, 2026 09:10
@efaulhaber efaulhaber requested review from LasNikas and svchb April 14, 2026 09:10
Comment thread src/neighborhood_search.jl Outdated
Comment thread src/neighborhood_search.jl Outdated
@LasNikas LasNikas enabled auto-merge (squash) April 14, 2026 15:18
@LasNikas LasNikas merged commit 11a445c into main Apr 14, 2026
15 checks passed
@LasNikas LasNikas deleted the ef/foreach-neighbor-unsafe branch April 14, 2026 15:40
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.

4 participants