Skip to content

Fail MMSeg evaluation on invalid ground truth - #3198

Open
dajiaohuang wants to merge 1 commit into
activeloopai:mainfrom
dajiaohuang:fix-mmseg-eval-mask-errors
Open

dajiaohuang wants to merge 1 commit into
activeloopai:mainfrom
dajiaohuang:fix-mmseg-eval-mask-errors

Conversation

@dajiaohuang

@dajiaohuang dajiaohuang commented Oct 2, 2026 •

Copy link
Copy Markdown

Fixes #3197.

MMSeg evaluation needs one ground-truth mask per prediction. Stop swallowing mask read/conversion failures in get_gt_seg_maps; the previous skip/reuse behavior could shift prediction-to-mask alignment or score an incorrect repeated mask. Raise the existing typed InvalidSegmentError with the mask column name.

Validation:

  • Focused pytest regressions: 2 passed, covering a failure at the first mask and after one valid mask.
  • Black check: passed.
  • git diff --check: passed.

The host does not have MMSeg/MMCV installed; the exact generator method was exercised from source with a mocked mask column.

Summary by CodeRabbit

  • Bug Fixes
    • Segmentation evaluation now reports an explicit error when a mask cannot be loaded or converted, instead of skipping it or reusing a previous mask.

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 1f5560ee-2448-474b-b02d-2730ca9c94de

📥 Commits

Reviewing files that changed from the base of the PR and between f432041 and 4718235.

📒 Files selected for processing (2)
  • python/deeplake/integrations/mmseg/mmseg_dataset_.py
  • python/tests/test_mmseg_eval_mask_errors.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

MMSegDataset.get_gt_seg_maps() now raises InvalidSegmentError when mask loading or upcasting fails. Tests cover failures at either of the first two mask positions.

Changes

MMSeg mask error handling

Layer / File(s) Summary
Raise and test mask errors
python/deeplake/integrations/mmseg/mmseg_dataset_.py, python/tests/test_mmseg_eval_mask_errors.py
The generator raises InvalidSegmentError with the mask tensor name and original exception when loading or upcasting fails. Tests check failures at indices 0 and 1.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 47182

Invalid ground-truth masks now stop evaluation rather than allowing misaligned scoring. No merge-blocking issue is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: MMSeg evaluation now fails when ground-truth masks are invalid.
Description check ✅ Passed The description explains the problem, the technical fix, the affected behavior, linked issue, validation results, and environment limitation. It does not explicitly mark the Bug fix impact checkbox, b…
Linked Issues check ✅ Passed The change satisfies #3197. MMSegDataset.get_gt_seg_maps now raises InvalidSegmentError(self.masks_tensor_name, e) when mask access or upcast_array fails. It no longer skips a mask or reuses a p…
Out of Scope Changes check ✅ Passed The reviewed changes are limited to the scoped get_gt_seg_maps error handling and focused regression tests. The tests directly verify the failure behavior required by #3197. No unrelated production …
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

A rabbit checks each mask in line,
No past map stands in for a sign.
If reading fails, an error shows,
The test confirms just where it goes.
Then hops away through fields of green.

Comment @coderabbitai help to get the list of available commands.

@khustup2

khustup2 commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

This changes behavior in a maintenance-only integration (MM/Labelbox/COCO) rather than fixing a defect, and it has no owner review or integration run behind it. Holding it: it needs the integration owner to decide whether the new behavior is wanted. No action needed from you now.

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.

MMSeg evaluation silently misaligns predictions when a ground-truth mask read fails

2 participants