Conversation
The two-pass np.unique deduplication in _match_detection_batch dropped valid TP assignments when a prediction's best-IoU target was already claimed by a higher-confidence prediction. The greedy one-pass algorithm (sort by IoU desc, assign if neither target nor pred already matched) was already used in _split_detections_by_outcome and ConfusionMatrix.evaluate_detection_batch but missing from Recall, F1Score, Precision, MeanAverageRecall, and MeanAveragePrecision. - Fix all five _match_detection_batch implementations - Add regression tests reproducing the issue #2378 example in each affected metric class (IoU matrix [[1.0, 0.667], [0.333, 0.538]]) --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2380 +/- ##
=======================================
+ Coverage 83% 83% +1%
=======================================
Files 69 70 +1
Lines 9628 9613 -15
=======================================
+ Hits 7946 7988 +42
+ Misses 1682 1625 -57 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes inconsistent TP/FP matching across detection metrics by replacing the np.unique-based deduplication in _match_detection_batch with a greedy one-pass matcher (aligning behavior with the existing greedy matcher used elsewhere in the metrics module) to prevent valid TP assignments from being dropped (issue #2378).
Changes:
- Replace
np.uniquetwo-pass deduplication with greedy IoU-desc matching in_match_detection_batchacross Recall, Precision, F1Score, MeanAverageRecall, and (deprecated)metrics.detection.MeanAveragePrecision. - Add regression tests covering the reported “two valid pairs” IoU-matrix case for Recall, Precision, F1Score, and MeanAverageRecall.
Review scores (n/5):
- Code quality: 4/5
- Testing: 3/5
- Docs: 4/5
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/supervision/metrics/recall.py |
Switch _match_detection_batch to greedy matching to avoid dropping valid TP pairs. |
src/supervision/metrics/precision.py |
Same greedy matching fix applied to Precision. |
src/supervision/metrics/f1_score.py |
Same greedy matching fix applied to F1Score. |
src/supervision/metrics/mean_average_recall.py |
Same greedy matching fix applied to MeanAverageRecall. |
src/supervision/metrics/detection.py |
Same greedy matching fix applied to deprecated MeanAveragePrecision._match_detection_batch. |
tests/metrics/test_recall.py |
Adds regression test reproducing #2378 matching failure for Recall. |
tests/metrics/test_precision.py |
Adds regression test reproducing #2378 matching failure for Precision. |
tests/metrics/test_f1_score.py |
Adds regression test reproducing #2378 matching failure for F1Score. |
tests/metrics/test_mean_average_recall.py |
Adds regression test reproducing #2378 matching failure for MeanAverageRecall. |
… test - Add kind='stable' to np.argsort in _match_detection_batch across all 5 metric implementations (Recall, Precision, F1Score, MeanAverageRecall, deprecated MeanAveragePrecision) to ensure deterministic TP assignment when IoU values tie - Add test_greedy_matching_two_valid_pairs to TestDetectionMetrics covering the deprecated MeanAveragePrecision._match_detection_batch with the issue #2378 IoU matrix, closing the one missing regression test flagged by /review --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
…cation Move the repeated 11-line greedy IoU matching block from all five _match_detection_batch implementations into a single shared private helper at supervision/metrics/utils/matching.py. Each call site reduces to two lines; empty-indices fast-path is handled by the generator yielding nothing. --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
--- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The two-pass np.unique deduplication in _match_detection_batch dropped valid TP assignments when a prediction's best-IoU target was already claimed by a higher-confidence prediction. The greedy one-pass algorithm (sort by IoU desc, assign if neither target nor pred already matched) was already used in _split_detections_by_outcome and ConfusionMatrix.evaluate_detection_batch but missing from Recall, F1Score, Precision, MeanAverageRecall, and MeanAveragePrecision.
resolves #2378