Skip to content

Validate and sort each channel once in compare_channels - #282

Open
raphaelvallat wants to merge 2 commits into
feat/interval-iou-comparisonfrom
compare-detection-prepared
Open

raphaelvallat wants to merge 2 commits into
feat/interval-iou-comparisonfrom
compare-detection-prepared

Conversation

@raphaelvallat

@raphaelvallat raphaelvallat commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Summary

compare_channels compares every pair of channels by calling the public yasa.compare_detection, which checks, sorts and removes duplicate events on every call. The events of each channel are therefore re-checked and re-sorted 2 × (n_chan - 1) times. This PR splits compare_detection into three private steps (_check_parameters, _prepare_events and _compare_prepared) so that compare_channels prepares each channel once, then only runs the matching for each pair. The public function and its outputs are unchanged.

On 64 channels with 5,000 events each, compare_channels goes from 6.3 s to 2.4 s with method="distance" (4.8 s to 4.7 s with method="intersection"), with identical scores.

One small behaviour change: compare_channels now validates iou_threshold and max_distance_sec before comparing any pair of channels. Before, invalid values were silently ignored on a single-channel detection, because no comparison was run.

Stacked on #278, through a copy of its branch in this repo (feat/interval-iou-comparison). Once #278 is merged, this PR will be retargeted to master.

AI assistance

Model used: Claude Opus 5.5 via Claude Code

Checklist

  • An issue was opened or discussed first (preferred)
  • The branch is up to date with master, and I checked open PRs for conflicts
  • The PR is small and focused, or stacked
  • The description starts with my own summary
  • Only necessary tests were added, and all tests pass
  • The documentation is updated
  • The changelog was not modified (it is updated by the maintainer before each release)

If AI-assisted:

  • The exact model used is reported
  • An AI review was run, the issues were fixed, and a summary was posted
  • /simplify was run (preferred, Claude Code)

🤖 Generated with Claude Code

Split yasa.compare_detection into _check_parameters, _prepare_events and
_compare_prepared. compare_channels now prepares each channel once and
calls _compare_prepared for every pair, instead of re-validating and
re-sorting (np.unique) the same events n_chan times. The public
function is unchanged.

64 channels x 5,000 events: distance 6.3 s -> 2.4 s, intersection
4.8 s -> 4.7 s, identical scores.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Oct 17, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.60%. Comparing base (0dbdd9a) to head (e48072f).

Additional details and impacted files
@@                      Coverage Diff                      @@
##           feat/interval-iou-comparison     #282   +/-   ##
=============================================================
  Coverage                         98.60%   98.60%           
=============================================================
  Files                                13       13           
  Lines                              3159     3165    +6     
  Branches                            429      431    +2     
=============================================================
+ Hits                               3115     3121    +6     
  Misses                               23       23           
  Partials                             21       21           

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

- compare_channels only applies np.unique (distance) on top of
  _events_by_channel, instead of re-validating each channel.
- _check_parameters no longer calls _check_method, so each entry point
  checks the method once.
- Document the sorted-input precondition of _compare_prepared.
- Test that parameters are validated even with a single channel.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@raphaelvallat

Copy link
Copy Markdown
Owner Author

AI review summary (/code-review with Claude Opus 5.5, on 784fe70)

No correctness issues found. Matching results are identical to compare_detection, which I confirmed on a 64-channel benchmark for both methods.

Fixed in e48072f:

  • Channels were validated twice in compare_channels. Now _events_by_channel handles validation, and compare_channels only removes duplicate start times in distance mode.
  • The method was checked twice per call. Each entry point now checks it once.
  • _compare_prepared didn't say that it needs sorted, unique distance indices. This is now in its docstring.
  • Invalid iou_threshold / max_distance_sec now raise even on single-channel detections. This is kept as a fix, tested, and noted in the description.

Not changed:

  • The compare_detection method still uses the public function. Removing duplicates up front would change n_self / n_other, and that method only runs one comparison per channel.
  • _match_intervals still calls argsort for every pair. The input is already sorted, so this costs about 0.08 s in total for 64 channels, which isn't worth a "sorted" flag.

🤖 Generated with Claude Code

@raphaelvallat
raphaelvallat changed the base branch from master to feat/interval-iou-comparison October 17, 2026 13:40
@raphaelvallat
raphaelvallat marked this pull request as ready for review October 17, 2026 13:41

This branch has not been deployed

No deployments
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