Skip to content

Fix slice index bug in NR ROI filter that excluded last event's final vertex#373

Merged
cfuselli merged 3 commits intonr_micro_clustering_carlofrom
copilot/sub-pr-372
Jan 29, 2026
Merged

Fix slice index bug in NR ROI filter that excluded last event's final vertex#373
cfuselli merged 3 commits intonr_micro_clustering_carlofrom
copilot/sub-pr-372

Conversation

Copy link

Copilot AI commented Jan 29, 2026

What does the code in this PR do / what does it improve?

Fixes a slice indexing bug in filter_events() where the last vertex of the last event wasn't marked for removal when the NR ROI check failed.

Can you briefly describe how it works?

The inner loop variable vertex_i has different meanings depending on loop exit:

  • Loop breaks: vertex_i points to next event's first vertex
  • Loop completes: vertex_i points to current event's last vertex

The original slice vertex_to_keep[start_index:vertex_i] worked for breaks but excluded the final vertex when the loop completed.

Fix: Track loop exit with boolean flag loop_broke and adjust end index accordingly:

for vertex_i in range(start_index, len(mps)):
    if _is_a_new_event:
        loop_broke = True
        break
    # ... process vertex

# If loop broke, vertex_i already points past current event
# If loop completed, need vertex_i + 1 to include last vertex
end_index = vertex_i if loop_broke else vertex_i + 1
vertex_to_keep[start_index:end_index] = 0

Can you give a minimal working example (or illustrate with a figure)?

Before fix - last event with 3 vertices fails ROI check:

mps = [(event=0, ...), (event=1, ...), (event=1, ...), (event=1, ...)]
# Indices:  0              1              2              3

filter_events(mps, ...)
# Returns: [1. 0. 0. 1.]  ❌ Index 3 incorrectly kept

After fix:

# Returns: [1. 0. 0. 0.]  ✓ All event 1 vertices removed

Please include the following if applicable:

  • Update the docstring(s)
  • Bump plugin version(s)
  • Update the documentation
  • Tests to check the (new) code is working as desired.
  • Does it solve one of the GitHub open issues?

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits January 29, 2026 13:59
Co-authored-by: cfuselli <62354392+cfuselli@users.noreply.github.com>
Co-authored-by: cfuselli <62354392+cfuselli@users.noreply.github.com>
Copilot AI changed the title [WIP] Address feedback on NR filter and logic updates Fix slice index bug in NR ROI filter that excluded last event's final vertex Jan 29, 2026
Copilot AI requested a review from cfuselli January 29, 2026 14:05
@cfuselli cfuselli marked this pull request as ready for review January 29, 2026 14:31
@cfuselli cfuselli merged commit 2ab01bf into nr_micro_clustering_carlo Jan 29, 2026
2 checks passed
@cfuselli cfuselli deleted the copilot/sub-pr-372 branch January 29, 2026 14:31
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.

2 participants