-
Notifications
You must be signed in to change notification settings - Fork 8
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
Update libCZI and fix shape error #124
Conversation
…etween boundingBox and logicalRect
result.emplace_back(m_statistics.sceneBoundingBoxes[scene_index_].boundingBoxLayer0); | ||
return result; | ||
} | ||
} |
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.
this method which uses boundingBoxLayer0 can return a different result than the following code which uses the subblock logicalRects. Preferring the logicalRect gives consistency between expected shape and read shape. Therefore I am removing this short-cut attempt to read the scene YX shape
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.
Hi Dan, I've reviewed the changes you've made and agree with them.
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 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.
The changes look reasonable to me. I thought there was a reason for it - the docs recommended the original method but I could be off on that. As long as the tests pass 👍
The bounds are definitely larger (1856) for a multichannel request because of that offset. ImageJ loads the data at 1848 (the actual plane size) instead of 1856. I haven't tested with bioformats. I feel comfortable that this code change is better for users who expect to just read the pixel data out of the czi file. |
We had an onprem image that bioio was badly failing to read.
Prior to this PR, bioio was reporting the shape as 5,60,180602,113778.
After updating libCZI, the shape was reported as 5,60,1248,1856. Much better! But the individual image planes were clearly sized at x=1848. It turned out that some of the channels had a different x offset than others (by 8 pixels), and the total x bounds spanned 1848+8 = 1856 pixels. Then when we go to load just one CZYX array from the data, there is a mismatch between the data shape and the expected total dask array shape. This would cause a crash inside bioio-czi.
So I also found and fixed the inconsistency here.
After this fix goes in, we have to do a release build of aicspylibczi, and then update bioio-czi to use it.