Concat all segments by default for multi-part masks
Resolves https://github.com/ultralytics/ultralytics/issues/16733
If the mask of an object is split into multiple segments, ultralytics currently returns the largest segment during conversion from mask to segments. But most users would rather the segments in this scenario are concatenated.
🛠️ PR Summary
Made with ❤️ by Ultralytics Actions
🌟 Summary
Refactored the masks2segments function for better clarity and usability.
📊 Key Changes
- Changed the
masks2segmentsfunction parameter fromstrategy(string) toconcat(boolean). - Updated code logic to reflect this change, simplifying conditional statements.
🎯 Purpose & Impact
-
Increased Clarity: The boolean parameter
concatmakes intended use clearer, enhancing usability for developers. - Simplification: Streamlines the code, making it easier to maintain and understand. This benefits both new and experienced users of the Ultralytics library.
👋 Hello @Y-T-G, thank you for submitting a PR to the ultralytics/ultralytics repository! 🚀
To facilitate a smooth integration of your contribution, please ensure you have covered the following checklist:
- ✅ Purpose Definition: Clearly articulate the purpose of your fix or feature in the PR description. Mention and link to any relevant issues you are addressing.
- ✅ Stay Updated: Ensure your branch is up-to-date with the
mainbranch. You can sync by selecting the 'Update branch' button or by runninggit pullandgit merge mainlocally. - ✅ Pass CI Checks: Verify that all Continuous Integration (CI) checks are successful. If any checks fail, make sure to resolve them promptly.
- ✅ Documentation Update: Reflect your changes in the relevant documentation to ensure users are well-informed of any new or adjusted features.
- ✅ Testing: Add or update tests as needed to validate your changes. Confirm that all tests are passing.
- ✅ Sign the CLA: If this is your first contribution, please sign our Contributor License Agreement by commenting “I have read the CLA Document and I sign the CLA.”
- ✅ Minimal Changes: Aim to keep your changes as focused as possible on the specific bug fix or feature addition. "It is not daily increase but daily decrease, hack away the unessential" — Bruce Lee
For further assistance, please refer to the Contributing Guide. If you have any questions, don't hesitate to ask. An Ultralytics engineer will be with you shortly to assist further. Thank you for contributing! 🌟
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 74.86%. Comparing base (
c5fd0bb) to head (f357c9e). Report is 3 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #16826 +/- ##
==========================================
- Coverage 74.87% 74.86% -0.01%
==========================================
Files 127 127
Lines 16845 16846 +1
==========================================
Hits 12612 12612
- Misses 4233 4234 +1
| Flag | Coverage Δ | |
|---|---|---|
| Benchmarks | 35.40% <25.00%> (-0.01%) |
:arrow_down: |
| GPU | 31.62% <25.00%> (-0.01%) |
:arrow_down: |
| Tests | 68.64% <100.00%> (-0.01%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
🚨 Try these New Features:
- Flaky Tests Detection - Detect and resolve failed and flaky tests
Agree with this change, there were multiple issues in the past with the same issue and this behavior makes more sense.
@Y-T-G Thanks for the PR! But in fact concatenating all the masks can result a wired mask like this https://github.com/ultralytics/ultralytics/issues/16733#issuecomment-2406365538 because all the coordinates might not be closed as a boundary of the object. And that's why we return the largest one, it might be a part of the whole object but the boundary is correct.
@Laughing-q That's true. There's an inherent limitation with the format not being able to represent masks that are split up. I guess we should at least show a warning when this behavior occurs.
@Laughing-q could create splits in segments by joining segment-groups with an float("nan") separator?
@Laughing-q could create splits in segments by joining segment-groups with an
float("nan")separator?
hmmm I tried but did not work to me as we need to scale coordinates to original image size domain and nan does not work with it, we could also add some logic to do this but it'll be a bit complicated.
@glenn-jocher @Y-T-G I think we can not satisfy every users cases and the largest option in fact works in most cases(in my opinion). Apparently there's way to get all the segments but need some customization on our code, which I would recommend we create and provide the customized code in our docs page. Then we can refer the docs to our users. What do you guys think?
something like:
from ultralytics.utils import ops
model = YOLO("yolo11n-seg.pt")
for r in model.predict():
segments = ops.masks2segments(r.masks.data, strategy="all") # each segment is a list
segments = [[ops.scale_coords(r.masks.shape[1:], x, r.orig_shape) for x in xy] for xy in segments]
We can deprecate concat since it usually returns distorted segments, and probably update it to all and return a list contains all the segments for each mask.
@Laughing-q It can work I guess. Should be better than concat.
Amazed this hasn't been addressed more thoroughly previously. Currently benchmarking YOLOv11 against another network and found very low segmentation performance, on closer inspection found that only the largest segmentations were returned. Pretty misleading to have result.masks only return a subset of the masks plotted using results.show()!
@Y-T-G @glenn-jocher Hey guys! actually we have a function merge_multi_segment to merge all the separate segments into one, that I created years ago to merge multiple segments from COCO dataset into one before converting to yolo format. And it works to the COCO2YOLO conversion, perhaps we could directly implement it here.
I've updated it in https://github.com/ultralytics/ultralytics/pull/16826/commits/c37c1a25807e086f852abeb89b62e1673372cc6a and tested it works to me. Can you test it as well? @Y-T-G THanks!
@Laughing-q Seems to work pretty well from my tests.
@Laughing-q PR merged! @Y-T-G thank you for the review and @wsd3val thank you for the feedback.
This should be live in 8.3.38 in the next 24 hours.