-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add multi-batch prediction for the segmentation and Polaris consumers #189
Conversation
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 tackling this change! I think it's a big improvement to functionality for segmentation.
Let me know how you want to handle the minor comments and then I'm happy to approve.
@@ -191,11 +195,11 @@ def _load_data(self, redis_hash, subdir, fname): | |||
labels = [frames[i] for i in range(num_frames)] | |||
|
|||
# Cast y to int to avoid issues during fourier transform/drift correction | |||
y = np.array(labels, dtype='uint16') | |||
y = np.expand_dims(np.array(labels, dtype='uint16'), axis=-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.
Is this essentially adding back a channel dimension?
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.
Yeah, the tracking model needs it to be 4D and the segmentation consumer is currently returning a 3D image (BXY)
# TODO: Why is there an extra dimension? | ||
# Not a problem in tests, only with application based results. | ||
# Issue with batch dimension from outputs? | ||
y = y[:, 0] if y.shape[1] == 1 else y | ||
# y = y[:, 0] if y.shape[1] == 1 else y |
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.
Thoughts on deleting vs. leaving commented out?
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.
I'm confused about when the 1st dimension would be 1 so I was worried that we were neglecting an edge case that would make this line relevant, but I don't feel strongly, we could delete it.
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.
Let's leave it just in case we end up debugging the edge case in the future.
Co-authored-by: Morgan Schwartz <[email protected]>
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.
LGTM!
This PR adds support for multi-batch images for the segmentation and Polaris consumers. The segmentation consumer receives a variable
dimension_order
from the frontend (see this PR). This variable has been added to the data generated by the Polaris consumer for segmentation jobs.We have also added more comprehensive testing of the segmentation consumer for input images of different dimension orders. This involved changes to the
DummyStorage
object to allow the dimensions of the images it generates to be specified.We have removed a line from the
get_image
utils function that expands the dimensions of loaded TIFF files by default, because this behavior complicated the handling of images with dimensions BXYC or CXYB. This change had little impact on the other functions that callget_image
, because this channel was often squeezed out immediately after loading.