Conversation
MeanAverageRecall matched every prediction first and then kept the top K by confidence. The matcher pairs by highest IoU, so a prediction ranked below K could take a target from one ranked within it. Match each detection limit's own top K predictions instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2604 +/- ##
=======================================
Coverage 91% 91%
=======================================
Files 78 78
Lines 11388 11396 +8
=======================================
+ Hits 10387 10395 +8
Misses 1001 1001 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The regression test does not exercise the claimed mAR@10 cutoff behavior.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes mAR@K by matching only each image’s top-K predictions before scoring.
Changes:
- Adds limit-specific matching.
- Adds a regression test and changelog entry.
- Assessment: Code 5/5, Testing 4/5, Docs 5/5.
File summaries
| File | Description |
|---|---|
src/supervision/metrics/mean_average_recall.py |
Implements top-K matching per detection limit. |
tests/metrics/test_mean_average_recall.py |
Tests low-confidence duplicate handling. |
docs/changelog.md |
Documents the corrected behavior. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+785
to
+797
| predictions = Detections( | ||
| xyxy=np.array([[0, 0, 10, 14], [0, 0, 10, 10]], dtype=np.float32), | ||
| confidence=np.array([0.9, 0.1]), | ||
| class_id=np.array([0, 0]), | ||
| ) | ||
|
|
||
| result = MeanAverageRecall().update(predictions, targets).compute() | ||
|
|
||
| # Only the top prediction counts at K=1, and its IoU of 100/140 matches the | ||
| # target at the thresholds 0.5 to 0.7, which are 5 of the 10. | ||
| assert result.mAR_at_1 == pytest.approx(0.5) | ||
| assert result.mAR_at_10 == pytest.approx(1.0) | ||
| assert result.mAR_at_100 == pytest.approx(1.0) |
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.
Description
MeanAverageRecallcan lower mAR@1 or mAR@10 when you add a prediction that ranks below the limit:The class docstring says "mAR @ 1 considers only the highest confidence detection for each image", and #2136 introduced per-image top-K for that reason. But
_computematches all of an image's predictions to its targets first, and only then drops the ones ranked below K (prediction_indices < max_detections). The shared matcher (_match_detection_batch_with_target_indices, #2380) is greedy by highest IoU, not by confidence. So in the example, the0.1duplicate with IoU1.0takes the target, the0.9prediction with IoU0.71is left unmatched, and then the duplicate is cut off at K=1. Nothing counts.pycocotools doesn't have this problem because it matches in score order, so a lower-ranked detection can never take a target from a higher-ranked one. With the IoU-first matcher, the limit has to be applied before matching.
This PR adds
_match_top_predictions, which runs the same matcher once per detection limit on that image's top K predictions and stores the result along a new last axis ofmatches,(P, Th, K)._compute_average_recall_for_classesthen reads the slice for each limit. Nothing else changes: not the matcher, not the per-image definition of K, and not the size buckets, which go through the same_compute.Motivation and Context
Duplicates are the typical trigger: after NMS, two overlapping predictions of the same object can both survive when their mutual IoU is below the NMS threshold, and the less confident one may fit the target more tightly. Crowded images with more than 10 predictions hit the same effect at mAR@10. A random check makes the scale visible. I generated 300 three-image scenes with jittered duplicates and compared
mAR_at_KagainstMeanAverageRecallrun on each image's top-K predictions only, which is the documented definition:developChanges Made
src/supervision/metrics/mean_average_recall.py: new_match_top_predictions._computesorts by confidence first and stores per-limit matches, and_compute_average_recall_for_classesindexes them by limit. It also gains a docstring.tests/metrics/test_mean_average_recall.py:test_mar_at_k_ignores_predictions_ranked_below_kis the example above: mAR@10.5, since the top prediction's IoU of100/140passes 5 of the 10 thresholds, and mAR@10 and mAR@1001.0.docs/changelog.md: entry under Unreleased.Testing
New test on
develop:With this change:
The existing mAR tests pass unchanged, including
test_complex_integration_scenarioand the per-image top-K tests from #2136.mypyon the changed files reports the same errors ondevelopand on this branch.docs/changelog.mdunderUnreleased(skip for lint/type/format-only or pure doc changes)Additional Notes
Matching now runs three times per image, once for each limit, instead of once. Each run only sees up to K predictions, so the extra cost is small next to the IoU matrix, which is computed once as before.
I left the matcher itself alone.
sv.match_detections,Precision,RecallandF1Scoreuse it on purpose without ranking by confidence, and they don't truncate by rank, so this problem doesn't apply to them.🤖 Generated with Claude Code