Add detection confidence curves - #429
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds a configurable bounding-box precision-recall curve metric, exposes pre-NMS detections from inference heads, and integrates scalar and image artifact logging into Lightning evaluation and MLflow key generation. ChangesPrecision-recall metric and contracts
Pre-NMS detection flow
Evaluation artifact logging
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ModelHead
participant non_max_suppression
participant PrecisionRecallCurve
participant LuxonisLightningModule
participant tracker
ModelHead->>non_max_suppression: produce bounding boxes from detections_pre_nms
ModelHead-->>PrecisionRecallCurve: return detections_pre_nms
PrecisionRecallCurve->>non_max_suppression: filter detections during update
PrecisionRecallCurve-->>LuxonisLightningModule: return computed curves and max_f1
LuxonisLightningModule->>PrecisionRecallCurve: get loggable values and artifacts
LuxonisLightningModule->>tracker: log metrics and curve image
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
luxonis_train/nodes/heads/precision_bbox_head.py (1)
110-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLift
_run_nmsintoBaseDetectionHeadinstead of duplicating the NMS kwargs.
EfficientBBoxHead._run_nmsnow encapsulates exactly this call with identical arguments. Both heads derive fromBaseDetectionHead, so hosting the helper there removes the drift risk between the two call sites.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@luxonis_train/nodes/heads/precision_bbox_head.py` around lines 110 - 121, Move the shared _run_nms implementation from EfficientBBoxHead into BaseDetectionHead, preserving the existing non_max_suppression arguments and behavior. Update the precision bbox head flow around _prepare_bbox_inference_output to call the inherited _run_nms helper instead of duplicating the NMS kwargs, and remove the subclass duplicate.luxonis_train/attached_modules/metrics/precision_recall_curve.py (1)
233-250: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPer-prediction
box_ioucalls make matching O(N) kernel launches per image.With
max_detup to 300 (default) per image and a large validation set, this launches one tiny IoU kernel per prediction. Computing the full IoU matrix once per image and then greedily consuming it is equivalent and much cheaper.♻️ Single IoU matrix per image
+ iou_matrix = ( + box_iou(prediction_boxes, target_boxes) + if len(target_boxes) + else prediction_boxes.new_zeros((len(prediction_boxes), 0)) + ) + class_match = ( + prediction_classes.unsqueeze(1) == target_classes.unsqueeze(0) + ) + for prediction_index in range(len(image_predictions)): - candidate_indices = torch.where( - (target_classes == prediction_classes[prediction_index]) - & ~matched_targets - )[0] + candidate_indices = torch.where( + class_match[prediction_index] & ~matched_targets + )[0] if candidate_indices.numel() == 0: continue - ious = box_iou( - prediction_boxes[prediction_index].unsqueeze(0), - target_boxes[candidate_indices], - ).squeeze(0) + ious = iou_matrix[prediction_index, candidate_indices] best_iou, best_local_index = torch.max(ious, dim=0)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@luxonis_train/attached_modules/metrics/precision_recall_curve.py` around lines 233 - 250, Update the matching logic around the prediction loop to compute one full IoU matrix between all prediction_boxes and target_boxes per image, then index that matrix for each prediction while greedily honoring matched_targets and the matching_iou_threshold. Remove the per-prediction box_iou call, preserving class filtering, best-candidate selection, and true_positive assignment.tests/unittests/test_metrics/test_precision_recall_curve.py (1)
285-337: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHarness duplicates
LuxonisLightningModuleinternals viaSimpleNamespace.Calling the unbound
_evaluation_epoch_end/get_mlflow_logging_keyswith a hand-rolled namespace means any added attribute access in those methods breaks these tests with an opaqueAttributeError. Acceptable here, but consider a small fixture that builds a real module (or aMock(spec=LuxonisLightningModule)) if this pattern spreads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unittests/test_metrics/test_precision_recall_curve.py` around lines 285 - 337, Update make_epoch_end_harness to construct a real LuxonisLightningModule or a Mock(spec=LuxonisLightningModule) instead of duplicating its internals with SimpleNamespace. Preserve the existing test-specific configuration, logged capture, and attributes needed by _evaluation_epoch_end and get_mlflow_logging_keys while allowing missing module attributes to be detected through the module interface.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@luxonis_train/attached_modules/metrics/precision_recall_curve.py`:
- Around line 33-53: Run docformatter on precision_recall_curve.py and apply its
formatting changes to the docstrings in __init__, compute, and get_artifacts,
preserving the documented content and behavior.
- Around line 342-348: Update get_artifacts so its runtime matplotlib import is
handled when the library is unavailable: either declare matplotlib in the
package’s runtime dependency metadata or catch the import failure and follow the
existing intended plotting-error or artifact-skipping behavior. Keep the curve
rendering path unchanged when matplotlib is installed.
In `@luxonis_train/lightning/luxonis_lightning.py`:
- Around line 862-874: Replace the ValueError raised in the artifact loop with
fail-soft handling: when an artifact’s dimension is not 3, log the contract
violation and skip that artifact instead of raising. Keep valid artifacts
flowing through self.tracker.log_image, and ensure the skip does not interrupt
the surrounding DDP execution.
---
Nitpick comments:
In `@luxonis_train/attached_modules/metrics/precision_recall_curve.py`:
- Around line 233-250: Update the matching logic around the prediction loop to
compute one full IoU matrix between all prediction_boxes and target_boxes per
image, then index that matrix for each prediction while greedily honoring
matched_targets and the matching_iou_threshold. Remove the per-prediction
box_iou call, preserving class filtering, best-candidate selection, and
true_positive assignment.
In `@luxonis_train/nodes/heads/precision_bbox_head.py`:
- Around line 110-121: Move the shared _run_nms implementation from
EfficientBBoxHead into BaseDetectionHead, preserving the existing
non_max_suppression arguments and behavior. Update the precision bbox head flow
around _prepare_bbox_inference_output to call the inherited _run_nms helper
instead of duplicating the NMS kwargs, and remove the subclass duplicate.
In `@tests/unittests/test_metrics/test_precision_recall_curve.py`:
- Around line 285-337: Update make_epoch_end_harness to construct a real
LuxonisLightningModule or a Mock(spec=LuxonisLightningModule) instead of
duplicating its internals with SimpleNamespace. Preserve the existing
test-specific configuration, logged capture, and attributes needed by
_evaluation_epoch_end and get_mlflow_logging_keys while allowing missing module
attributes to be detected through the module interface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 503af074-6e78-48fe-a38b-3aa889a8f9d5
📒 Files selected for processing (7)
luxonis_train/attached_modules/metrics/__init__.pyluxonis_train/attached_modules/metrics/base_metric.pyluxonis_train/attached_modules/metrics/precision_recall_curve.pyluxonis_train/lightning/luxonis_lightning.pyluxonis_train/nodes/heads/efficient_bbox_head.pyluxonis_train/nodes/heads/precision_bbox_head.pytests/unittests/test_metrics/test_precision_recall_curve.py
rolandocortez
force-pushed
the
feat/detection-confidence-curves
branch
2 times, most recently
from
July 28, 2026 12:47
5fdfe6e to
3e943db
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
luxonis_train/attached_modules/metrics/precision_recall_curve.py (1)
14-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueApprove with a minor static-analysis nit on
supported_tasks.Ruff RUF012 flags the mutable list default on
supported_tasks. Low value here since the logic is unaffected (this attribute is never mutated), but consider aClassVar[list[Tasks]]annotation or a tuple to silence the hint if the codebase already does this elsewhere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@luxonis_train/attached_modules/metrics/precision_recall_curve.py` around lines 14 - 21, Update the class-level supported_tasks declaration in PrecisionRecallCurve to avoid Ruff RUF012, using the codebase’s established convention: annotate it as ClassVar[list[Tasks]] or make it an immutable tuple while preserving the BOUNDINGBOX task value.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@luxonis_train/lightning/luxonis_lightning.py`:
- Around line 881-910: Wrap each per-artifact image conversion and
self.tracker.log_image call in a try/except within the is_global_zero block,
logging the exception with metric and artifact context and continuing to the
next artifact instead of propagating it. Preserve the existing shape validation
and successful logging behavior.
---
Nitpick comments:
In `@luxonis_train/attached_modules/metrics/precision_recall_curve.py`:
- Around line 14-21: Update the class-level supported_tasks declaration in
PrecisionRecallCurve to avoid Ruff RUF012, using the codebase’s established
convention: annotate it as ClassVar[list[Tasks]] or make it an immutable tuple
while preserving the BOUNDINGBOX task value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a0de5c4c-21b5-46f5-9608-8f0e0b0ab7ef
📒 Files selected for processing (8)
luxonis_train/attached_modules/metrics/__init__.pyluxonis_train/attached_modules/metrics/base_metric.pyluxonis_train/attached_modules/metrics/precision_recall_curve.pyluxonis_train/lightning/luxonis_lightning.pyluxonis_train/nodes/heads/efficient_bbox_head.pyluxonis_train/nodes/heads/precision_bbox_head.pyrequirements.txttests/unittests/test_metrics/test_precision_recall_curve.py
🚧 Files skipped from review as they are similar to previous changes (5)
- luxonis_train/attached_modules/metrics/init.py
- luxonis_train/attached_modules/metrics/base_metric.py
- luxonis_train/nodes/heads/efficient_bbox_head.py
- luxonis_train/nodes/heads/precision_bbox_head.py
- tests/unittests/test_metrics/test_precision_recall_curve.py
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #429 +/- ##
=======================================
Coverage ? 93.89%
=======================================
Files ? 271
Lines ? 13582
Branches ? 0
=======================================
Hits ? 12753
Misses ? 829
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
rolandocortez
force-pushed
the
feat/detection-confidence-curves
branch
from
July 28, 2026 15:28
3e943db to
8cfca72
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@luxonis_train/lightning/luxonis_lightning.py`:
- Around line 901-910: Wrap the artifact conversion and self.tracker.log_image
call in the existing per-artifact fail-soft handling so conversion or tracker
backend errors are caught and do not escape on rank zero or deadlock other DDP
ranks; preserve processing of subsequent artifacts. Add a regression test
alongside the existing get_artifacts-failure and invalid-shape tests that makes
tracker.log_image fail and verifies the failure is handled without propagating.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eaaf1bfa-b66f-4ef8-9a46-aa4e591e887c
📒 Files selected for processing (8)
luxonis_train/attached_modules/metrics/__init__.pyluxonis_train/attached_modules/metrics/base_metric.pyluxonis_train/attached_modules/metrics/precision_recall_curve.pyluxonis_train/lightning/luxonis_lightning.pyluxonis_train/nodes/heads/efficient_bbox_head.pyluxonis_train/nodes/heads/precision_bbox_head.pyrequirements.txttests/unittests/test_metrics/test_precision_recall_curve.py
🚧 Files skipped from review as they are similar to previous changes (6)
- luxonis_train/attached_modules/metrics/init.py
- requirements.txt
- luxonis_train/attached_modules/metrics/base_metric.py
- tests/unittests/test_metrics/test_precision_recall_curve.py
- luxonis_train/nodes/heads/efficient_bbox_head.py
- luxonis_train/nodes/heads/precision_bbox_head.py
kozlov721
reviewed
Jul 29, 2026
kozlov721
self-requested a review
July 31, 2026 14:51
kozlov721
approved these changes
Jul 31, 2026
klemen1999
approved these changes
Aug 4, 2026
kozlov721
force-pushed
the
feat/detection-confidence-curves
branch
from
August 4, 2026 16:39
825c06d to
9bd8bdb
Compare
Merged
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
3 participants
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.
Add precision-recall and confidence curves for detection
Purpose
Add confidence-based evaluation curves for bounding-box detection in
luxonis-train.The new metric provides:
The current detection metrics operate on post-NMS detections produced at a fixed confidence threshold. That is sufficient for scalar metrics such as mAP, but it does not provide the decoded candidates needed to evaluate how precision and recall change across confidence thresholds.
This PR adds an opt-in
PrecisionRecallCurvemetric and exposes decoded pre-NMS detections from the supported bounding-box heads during evaluation.Specification
Detection-head output
Updated:
EfficientBBoxHeadPrecisionBBoxHeadBoth heads now expose:
boundingboxoutputdetections_pre_nmstensor during evaluationTraining and export output contracts remain unchanged.
For
EfficientBBoxHead, decoding and NMS are separated into:_prepare_bbox_inference_output()_run_nms()The existing
_postprocess_detections()behavior is preserved as a wrapper around those two steps.PrecisionRecallCurve metric
Added an opt-in
PrecisionRecallCurvemetric for bounding-box detection.The metric:
The metric computes:
max_f1is exposed as the primary scalar under the metric name.confidence_at_max_f1is exposed as a submetric.Metric artifacts
Extended the metric interface with generic hooks for:
PrecisionRecallCurvelogs one RGB image containing:The artifact is generated only on global rank zero.
Scope
This PR intentionally supports:
The following are not included:
mAP remains the recommended primary detection metric.
Dependencies & Potential Impact
No new external dependency is introduced.
Potential impact:
EfficientBBoxHeadandPrecisionBBoxHeadnow include the additionaldetections_pre_nmskeyboundingboxoutput remains unchangedThe metric uses the decoded candidates produced by the detection head, so the supported heads and the metric share the same pre-NMS tensor contract.
Testing & Validation
Validation performed:
git diff --checkpassedFocused metric validation:
PrecisionRecallCurvetests passedlog_sub_metrics=TrueandFalsecoveredDetection-head validation:
EfficientBBoxHeadPrecisionBBoxHeaddetections_pre_nmsRegression validation:
Distributed validation:
The final commit was also rebased onto the current
mainand revalidated before push.Deployment Plan
No deployment is required.
Rollout:
PrecisionRecallCurveonly in detection configurations that require confidence-based curvesRollback:
Monitoring:
AI Usage
Assisted-by: ChatGPT
Summary by CodeRabbit
New Features
Improvements
Tests
Chores