-
Notifications
You must be signed in to change notification settings - Fork 239
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
Lots of Zeiss CZI fixes #1078
Lots of Zeiss CZI fixes #1078
Conversation
This is mostly to assist in debugging issues with very large files.
The stored Y coordinate is the actual Y coordinate; it doesn't need to be subtracted from the image height. Fixes ticket #11367.
This uses the stored tile row/column instead of assuming that tiles were acquired in comb mode. It also prevents tiles from being copied to the wrong row when the image width is greater than the sum of the tile widths. Fixes #11784.
The channel count and interleaved values were previously set incorrectly; openBytes also did not account for the fact that RGB images are stored in BGR order. Fixes #11233.
Fixes #11784.
Fixes #11166.
These are really just packed pixels; 12 bits are stored for 16-bit (12 valid bit) data. Some transposing needs to occur type 504, though it's not completely clear to me why. Fixes #11204, fixes #9398.
Also updates the relevant test to check for the new behavior. Fixes #12189.
This lets us set a buffer size for a specific stream/handle, instead of calling the static setDefaultBufferSize method in NIOFileHandle.
We have to do a lot of seeking and opening new streams to read a small number of bytes; the only place where large chunks of bytes are read is when the pixel data is processed. This should speed up setId times, as we're not attempting to buffer 1MB every time a new stream is opened (potentially millions of times for very large datasets).
...instead of throwing an exception. Many of the camera-dependent encoding types are just raw data, and were correctly handled previously.
The same values are stored elsewhere, and for very large datasets there can be potentially tens of thousands of DisplaySetting entries.
Conflicting PR. Removed from build OMERO-5.0-merge-daily#650. See the console output for more details. |
Conflicting PR. Removed from build OMERO-5.0-merge-daily#651. See the console output for more details. |
gh-1018 had an erroneous .czi-related commit which conflicted with this, and has now been removed. |
Re: http://trac.openmicroscopy.org.uk/ome/ticket/11367 (the ROI ticket): Questions:
Pre-upgrade ImageJ (this PR not in) ImageJ with the Insight (with this PR in) |
Re: http://trac.openmicroscopy.org.uk/ome/ticket/12124 Test passes in ImageJ. (see #1078 (comment) - the questions there are the same). Oritinal xml |
Note: Nr. 2 and the last one in this list http://trac.openmicroscopy.org.uk/ome/query?status=accepted&status=closed&status=new&status=reopened&keywords=%7Eczi&col=id&col=summary&col=status&col=type&col=priority&col=milestone&col=component&order=priority will be done by Balaji. Note: check the list on Trello card to see what is done already - tick off as you are progressing. |
Re: http://trac.openmicroscopy.org.uk/ome/ticket/11204. Compared in Insight on Gretzky (merge, with this PR in) and Nightshade, with the Insight from "latest" (this PR not in). When the QA 7405 image was imported on Gretzky, there were structures visible. On Nightshade, the image (multi-z, all planes) was black. This seems to work fine. |
Re: http://trac.openmicroscopy.org.uk/ome/ticket/11233. Imported to Gretzky as user-1, Project czi testing 11233, ID 40813. This is with this PR in. The image looks like it should comparing to the png submitted with it (also imported in the same Project). Compare with Nightshade and "latest" Insight - one image looks black (without this PR). There is one black image. Also imported the 7601 QA and the corresponding images from @rleigh-dundee 's team folder - all look fine. Nevertherless the 7891 QA image from Apache is not okay - it says "not a valid image in Insight" - see gretzky, user-1 - Project as above, ID 40869 The last problem (the line above, 7891 QA) is resolved - it was just building pyramids probably. Now it displays fine. This seems to work fine then. |
@bramalingam @rleigh-dundee @joshmoore : We had a discussion regarding testing of http://trac.openmicroscopy.org.uk/ome/ticket/12189 - the one with ~760 G of data to import. @joshmoore suggested that the solution would be an inplace import to gretzky, but @manics is telling me such an in-place is not possible as squig is not mounted on gretzky. @joshmoore ? |
Hi Petr, Had a discussion with Melissa on this regard. The server side operations of bio-formats should not affect this dataset. Best, Mr Balaji Ramalingam Software Developer OME Team College of Life Sciences University of Dundee From: pwalczysko <notifications@github.commailto:notifications@github.com> @bramalingamhttps://github.com/bramalingam@rleigh-dundeehttps://github.com/rleigh-dundee@joshmoorehttps://github.com/joshmoore : We had a discussion regarding testing of http://trac.openmicroscopy.org.uk/ome/ticket/12189 - the one with ~760 G of data to import. @joshmoorehttps://github.com/joshmoore suggested that the solution would be an inplace import to gretzky, but @manicshttps://github.com/manics is telling me such an in-place is not possible as squig is not mounted on gretzky. @joshmoorehttps://github.com/joshmoore ? Reply to this email directly or view it on GitHubhttps://github.com//pull/1078#issuecomment-42425060. The University of Dundee is a registered Scottish Charity, No: SC015096 |
@bramalingam : Okay, I will leave this with you then, thanks. |
@rleigh-dundee is dealing with http://trac.openmicroscopy.org.uk/ome/ticket/11784, waiting for pyramid generation atm (user-4/czi) |
11784 looks fine here. |
Also checked http://trac.openmicroscopy.org.uk/ome/ticket/9398. This works fine (was black on nightshade without this PR, is identical with the "export" image on gretzky with this PR. Adding to Trello and ticking off. |
For trac 11166:
and RefractiveIndex* are still NaNs. It's like that in the original metadata, but is that a computed value or actually what it is in the CZI? The Channel warning looks like a bug though. BPAE-cells_mitosis_3chZ(WF).czi is OK. |
I thought I had noted it in the ticket, but for ticket 11166, NaN really is what is stored for the RefractiveIndex (almost all metadata values are parsed from blocks of not-quite-OME-XML). The Channel warning should be safe to ignore assuming there are no OME-XML validation errors, and that the file successfully imports into OMERO. |
http://trac.openmicroscopy.org.uk/ome/ticket/12189 The grouping error still seems to persist, OMERO Version: 5.0.1-rc1-ice33-b20 Import Client : Matlab based Inplace import.. Stack trace while pointing at the top directory, FILE_EXCEPTION: /ome/data_repo/from_Biozentrum_Basel/Charles/SPIM/large_timeseries_700GB/Image 17-24hpf-46hpf(1).czi The importer continues to read every individual file within the folder, which was previously the case as well, and if allowed to continue imports every individual file within the folder separately (T grouping == not right). |
Last two commits should help with the file grouping. If the grouping is still wrong with the current state of this branch, then could you please point me at the Matlab client being used? |
Was there anything left here besides the file grouping issue noticed by @bramalingam? |
Not that I can tell. |
The T grouping was the only issue. Dint notice anything else. |
I think the issues dealt with by @bramalingam were the last outstanding here, see https://trello.com/c/N6Wxa2UX/39-czi-bugs-5-0-2-blockers |
|
@melissalinkert : Still reading individual files? Client version and bioformats version mentioned in the report. Allowing the upload to go through to check the output. |
More updates : Server not upgraded. Demo server being used to upload. Its the stable release : 5.0.1 |
The above shows 4876596 as being the Bio-Formats commit hash, which is the current |
As far as I can tell |
The omero_client was yesterdays build. Because the recent omero_client.jar (including Blazej's PR had some errors.
So the omero_client.jar was yesterday's. The Bioformats_package.jar was today's. |
It's not clear to me what would cause that exception, though my suspicion is that it's not related to this PR. It sounds like you haven't been using true merge builds (i.e. jars have been copied/replaced), which we know can cause problems, so maybe the thing to do is to merge this and re-test with RC1 client and server? Any real blocker can be fixed in an emergency PR. |
With everyone away this afternoon, and the issues above being primarily deployment issues (that are fully my fault!), I'd say let's merge this and get bump the submodule. @bramalingam, if your testing shows any issues, please raise another |
Merging with visual thumbs up from @melissalinkert and @sbesson. |
--rebased-to #1138 |
See http://trac.openmicroscopy.org.uk/ome/query?status=accepted&status=closed&status=new&status=reopened&keywords=~czi&col=id&col=summary&col=status&col=type&col=priority&col=milestone&col=component&order=priority.
All tickets listed there should be resolved; the relevant files from each ticket will need to be imported and/or opened in ImageJ to verify that the import succeeds, images look correct, ROI data is correct (where applicable to the ticket), and that the generated OME-XML is valid. Most likely this will need multiple pairs of eyes, as having one person do all of the imports and validation may be burdensome.
See also the Trello card (https://trello.com/c/N6Wxa2UX/39-czi-bugs-5-0-2-blockers), which has a draft for announcing this to the community ahead of 5.0.2.
/cc @bramalingam, @emilroz (since you've both previously reported issues with this data)