Skip to content

Add proper support for inactive points#107

Merged
efaulhaber merged 17 commits intomainfrom
ef/inactive-particles
May 13, 2025
Merged

Add proper support for inactive points#107
efaulhaber merged 17 commits intomainfrom
ef/inactive-particles

Conversation

@efaulhaber
Copy link
Copy Markdown
Member

No description provided.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 17, 2025

Codecov Report

Attention: Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.02%. Comparing base (a65dc9a) to head (9ba598c).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/nhs_grid.jl 83.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #107      +/-   ##
==========================================
- Coverage   89.27%   89.02%   -0.25%     
==========================================
  Files          13       13              
  Lines         550      556       +6     
==========================================
+ Hits          491      495       +4     
- Misses         59       61       +2     
Flag Coverage Δ
unit 89.02% <86.66%> (-0.25%) ⬇️

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 self-assigned this Apr 17, 2025
@efaulhaber efaulhaber marked this pull request as ready for review May 13, 2025 13:13
@efaulhaber efaulhaber requested review from LasNikas and Copilot May 13, 2025 13:14
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 adds support for inactive points in neighborhood searches by introducing a new keyword parameter that restricts the active indices from the provided coordinate arrays. Key changes include:

  • Updating test cases to verify behavior with inactive points using a specified index range.
  • Modifying various neighborhood search initialization and update functions (in both grid and precomputed implementations) to accept the new eachindex_y parameter.
  • Adjusting the core grid search functions to enforce constraints on inactive point handling.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/nhs_grid.jl Updated tests to pass a specific indices range (eachindex_y) for inactive points.
src/nhs_trivial.jl Added eachindex_y parameter to the trivial search initialization and update functions.
src/nhs_precomputed.jl Introduced eachindex_y in initialization and update functions and propagated it to neighbor list setup.
src/nhs_grid.jl Modified grid initialization and update functions to support selective point processing with eachindex_y.
src/neighborhood_search.jl Extended abstract functions to include eachindex_y for proper inactive point support.
Comments suppressed due to low confidence (2)

src/nhs_trivial.jl:37

  • [nitpick] Consider renaming the parameter 'eachindex_y' to a more descriptive name like 'active_indices' to clarify its role in specifying which points to use.
eachindex_y = axes(y, 2)

src/nhs_grid.jl:215

  • [nitpick] Verify that the added bounds check does not introduce significant performance overhead in critical sections; consider conditional enabling for production builds if needed.
@boundscheck checkbounds(y, eachindex_y)

Comment thread src/nhs_grid.jl
Copy link
Copy Markdown
Collaborator

@LasNikas LasNikas left a comment

Choose a reason for hiding this comment

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

Thanks a lot! LGTM!

@efaulhaber efaulhaber merged commit d5d79da into main May 13, 2025
10 of 12 checks passed
@efaulhaber efaulhaber deleted the ef/inactive-particles branch May 13, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants