Fix confidence handling for keypoint evaluation #5344
Conversation
brimoor
requested review from
mwoodson1,
manushreegangwar,
ehofesmann and
jacobsela
WalkthroughThe pull request introduces modifications to the evaluation utilities in FiftyOne, specifically focusing on enhancing the handling of predicted objects, particularly keypoints, in COCO and OpenImages evaluation processes. The changes primarily involve updating the sorting and confidence calculation logic to accommodate Changes
Sequence DiagramsequenceDiagram
participant Evaluator
participant Predictions
participant MatchingLogic
Evaluator->>Predictions: Sort predictions
alt Is Keypoints
Predictions-->>Evaluator: Sort by mean keypoint confidence
else Regular Predictions
Predictions-->>Evaluator: Sort by prediction confidence
end
Evaluator->>MatchingLogic: Compute matches
alt Is Keypoints
MatchingLogic-->>Evaluator: Use np.nanmean(confidence)
else Regular Predictions
MatchingLogic-->>Evaluator: Use direct confidence
end
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
fiftyone/utils/eval/coco.py (1)
556-560: Consider replacinglambdawith a regular function
Per static analysis suggestions, defining a named function is often considered more readable and debuggable than assigning alambda.-if isinstance(preds, fol.Keypoints): - sort_key = lambda p: np.nanmean(p.confidence) if p.confidence else -1 -else: - sort_key = lambda p: p.confidence or -1 +def _sort_key_for_keypoints(p): + return np.nanmean(p.confidence) if p.confidence else -1 +def _sort_key_for_detections(p): + return p.confidence or -1 if isinstance(preds, fol.Keypoints): sort_key = _sort_key_for_keypoints else: sort_key = _sort_key_for_detections🧰 Tools
🪛 Ruff (0.8.2)
557-557: Do not assign a
lambdaexpression, use adefRewrite
sort_keyas adef(E731)
559-559: Do not assign a
lambdaexpression, use adefRewrite
sort_keyas adef(E731)
fiftyone/utils/eval/openimages.py (1)
549-553: Consider replacinglambdawith a regular function
Replacing thelambdadefinitions with named functions can improve readability, as recommended by static analysis.-if isinstance(preds, fol.Keypoints): - sort_key = lambda p: np.nanmean(p.confidence) if p.confidence else -1 -else: - sort_key = lambda p: p.confidence or -1 +def _sort_key_for_keypoints(p): + return np.nanmean(p.confidence) if p.confidence else -1 +def _sort_key_for_detections(p): + return p.confidence or -1 if isinstance(preds, fol.Keypoints): sort_key = _sort_key_for_keypoints else: sort_key = _sort_key_for_detections🧰 Tools
🪛 Ruff (0.8.2)
550-550: Do not assign a
lambdaexpression, use adefRewrite
sort_keyas adef(E731)
552-552: Do not assign a
lambdaexpression, use adefRewrite
sort_keyas adef(E731)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
fiftyone/utils/eval/coco.py(6 hunks)fiftyone/utils/eval/openimages.py(5 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
fiftyone/utils/eval/openimages.py
550-550: Do not assign a lambda expression, use a def
Rewrite sort_key as a def
(E731)
552-552: Do not assign a lambda expression, use a def
Rewrite sort_key as a def
(E731)
fiftyone/utils/eval/coco.py
557-557: Do not assign a lambda expression, use a def
Rewrite sort_key as a def
(E731)
559-559: Do not assign a lambda expression, use a def
Rewrite sort_key as a def
(E731)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.11)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.10)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.9)
- GitHub Check: e2e / test-e2e
- GitHub Check: build
🔇 Additional comments (12)
fiftyone/utils/eval/coco.py (6)
16-16: Import added for keypoint handling
This import allows proper recognition ofKeypointsfor COCO evaluation.
568-568: Sorting predictions by confidence
Sorting by descending confidence is a necessary step for COCO-style matching and looks correct.
596-602: Keypoint confidence logic
Usingnp.nanmeanforKeypointconfidences is an effective approach to handle multiple values. The rest of the fallback logic seems correct.
654-654: Recording prediction confidence in match
Includingpred_confin the stored match data is correct and ensures consistent calculations.
667-667: Recording unmatched object as false positive
Keepingpred_confin the appended match record clarifies the final confidence used, which is good for debugging.
681-681: Recording false positive for unmatched predictions
This line correctly savespred_confalongside the unmatched prediction label.fiftyone/utils/eval/openimages.py (6)
13-13: Import added for keypoint handling
This import line matches the changes needed to properly processKeypoints.
561-561: Sorting predictions
Sorting in descending confidence is essential for the matching approach and is correctly implemented.
592-598: HandlingKeypointvs detection confidences
Averaging multiple keypoint confidences withnp.nanmeanensures robust evaluation. The fallback for regular detections is consistent.
683-683: Storing confidence in crowd or matched scenario
Preservespred_confin the matches for further reporting, which is beneficial for debugging.
695-695: Unmatched prediction labeled false positive
Recording thepred_conffor unsuccessful matches is valuable for deeper analysis.
703-703: Persisting confidence for leftover predictions
Maintainingpred_confin the final data ensures consistent logging of confidence values for unmatched objects.
Change log
evaluate_detections()would fail when applied toKeypointsfieldsconfidenceis a list, not a scalar, for keypointsExample usage
Summary by CodeRabbit
New Features
Bug Fixes
Improvements