[WIP] Dense Mask IoU #5283
[WIP] Dense Mask IoU #5283
Conversation
|
Important Review skippedIgnore keyword(s) in the title. ⛔ Ignored keywords (3)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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
|
|
@ehofesmann I know you mentioned there would be some edge cases regarding different pred and ground truth resolutions. Do you think my implementation covers this sufficiently? If not can you provide a small test case I can use? |
Very nice work! I haven't run this myself, but I do believe that this todo would need to be addressed in order to support preds and gts with different resolutions. You could try to test it with something like: gt_mask = np.zeros((90,90))
gt_mask[:60,:] = 1
pred_mask = np.zeros((50,50))
pred_mask[25:,:] = 1
gt = fo.Detection(bounding_box=[0.1,0.1,0.1,0.1], label="test", mask=gt_mask)
pred = fo.Detection(bounding_box=[0.1,0.1,0.1,0.1], label="test", mask=pred_mask)
fo.utils.iou._dense_iou(pred, gt) |
|
I'm not sure I understand the use case here. When do we have predictions and ground truths of different resolutions? And for the example you shared what is the expected behaviour |
I could definitely imagine a case where the ground truth labels were annotated on higher res images than what a segmentation model results. And also that 2 segmentation models might have resulting masks of different resolutions and you want to compare one with the other. For the example I would expect there to be an overlap of 1/6 (one mask covers the bottom 2/3 of a square image and the other mask the top 1/2) |
|
Got it, this should be handled correctly now. |
There was a problem hiding this comment.
Code LGTM from a static review standpoint.
Note: I haven't run this myself; relying on you for implementation correctness.
Just to double confirm: as @ehofesmann pointed out, FO does not enforce nor assume that the masks for each object have the same resolution, so the IoU implementations must gracefully handle two Detection instances whose masks are different sizes.
I added some documentation for this change here: 79080ff
What changes are proposed in this pull request?
Now when calling
fo.utils.iou.compute_ious(preds, gts, tolerance=None)with pixel masks, the IoU will be computed using pixels instead of converting to contours. This is especially needed when trying to evaluate segmentation models which typically use dense IoU as a metric instead of one computed on contours.How is this patch tested? If it is not, please explain why.
(Details)
Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)
What areas of FiftyOne does this PR affect?
fiftyonePython library changes