-
Notifications
You must be signed in to change notification settings - Fork 602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change Mask to use coco-style segmentation by default #888
Conversation
When I do instance segmentation prediction, the code reports an error. But my prediction using the official version was successful: Traceback (most recent call last): Process finished with exit code 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
conflicts need to be resolved
Finally got time to work on this. Only breaking change that I found is that each time we use This also adds |
This code still uses bounding boxes in the post processing to merge the tiles, this results in cutoff masks since masks inside a box might be removed. A better approach would be to look at the masks polygon and calculate IoU etc from those. Here is a paper: https://isprs-annals.copernicus.org/articles/V-2-2022/291/2022/ which also includes some examples of the current solutions problem. |
@Preburk good point, probably should try to implement NMS for polygons. Using shapely should work cause IoU is just def poly_IoU(poly_1:Polygon, poly_2: Polygon) -> float:
area_intersection = poly_1.intersection(poly_2).area
area_union = poly_1.union(poly_2).area
iou = area_intersection / area_union
return iou Though have to more accurately check how merging the slices is handled in sahi. |
Shapely is too slow; one should first use bounding boxes to find all intersecting targets, and then use Shapely |
Hey @mayrajeo this pr's commit history contains uncompressed large file commits as https://github.com/obss/sahi/blob/e114ee74a6ff4795ad62ab5a28c7244c49ae1d20/demo/demo_data/prediction_visual.png, can you please reopen a new pr without the large image file? I will gladly accept your PR, it looks very promising 💯 |
@@ -0,0 +1,5 @@ | |||
2 0.3212890625 0.7749266862170088 0.216796875 0.23607038123167157 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these files should not be commited
@@ -0,0 +1,7 @@ | |||
names: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are lots of unnecessary files :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll fix unnecessary file conflicts and open a new one. After it's added someone can figure how to implement NMS/NMM/GREEDYNMM for polygons efficiently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fast response! @mayrajeo
You can ignore the polygon postprocess in your pr, I will try to handle it in a separate pr 👌🏻
Continues in #1039 |
I suggest that instead of numpy arrays, masks are saved either as shapely polygons or COCO-style annotations. The reason for this is that for large images, such as Sentinel 2 satellite tiles (10980x10980 pixels), each time mask is shifted and
Mask.get_shifted_mask()
is called, sahi creates a new empty array that is same size that the original image. This takes a lot of time compared to shifting just the coordinates withFor example, in my use case I'm slicing 10980x10980 tiles into 320x320px slices with overlap of 0.2, so around 1850ish tiles. Shifting each predicted mask took around one second, and these images can contain several hundreds if not over two thousand objects of interest, meaning that just shifting the masks will take several times longer than getting the actual predictions.
Merging might needs a bit of fixing, as I'm not really happy how the code looks but it should work like current implementation. I briefly considered using rle for this but they require boolean masks first and creating them is slow.
Just for the note:
.buffer(0)
is a quickhack to fix somehow invalid polygons, and if-else ensures that only polygons (not lines or points) are used to constructMultiPolygon
.I hope that I found all places that this change affects. For
models
, this means constructingObjectPrediction
withsegmentation
instead ofbool_mask
. Another way is create a methodObjectPrediction.from_bool_mask
and use it.