mobie / mobie-viewer-fiji

BSD 2-Clause "Simplified" License
30 stars 12 forks source link

OME.ZARR writer #360

Closed tischi closed 3 years ago

tischi commented 3 years ago

I think it would relatively high priority to also be able to write OME.ZARR from Java soon. Anything that we could look forward to from the OME side (e.g., Java API for bioformats2raw)?

I think @joshmoore and @K-Meech looked into it at some point but maybe it was not yet working?

cc @constantinpape @KateMoreva @jburel

K-Meech commented 3 years ago

Linking this issue as there was some discussion of this there: https://github.com/mobie/mobie-viewer-fiji/issues/149#issuecomment-743073779

K-Meech commented 3 years ago

@joshmoore made some suggestions a while ago, but I never got around to testing them out properly. Happy to look into this again now / integrate with the mobie project creation code! At the time it looked like bioformats2raw would be the easiest solution.

K-Meech commented 3 years ago

@KateMoreva @constantinpape - is there a command for opening ome-zarr from the filesystem yet? (this would be very useful for testing - no worries if not!)

joshmoore commented 3 years ago

With me away, double posting questions on image.sc or @ mentioning @dgault is probably safest.

KateMoreva commented 3 years ago

@KateMoreva @constantinpape - is there a command for opening ome-zarr from the filesystem yet? (this would be very useful for testing - no worries if not!)

Yes, there is Plugins>BigDataViewer>OME ZARR>Open OME ZARR from the file system... and MoBIE version in MR 350

K-Meech commented 3 years ago

Thanks @KateMoreva! Is this command for OME-ZARR, or bdv style OME-ZARR (i.e. does it need an xml file too)?

KateMoreva commented 3 years ago

Thanks @KateMoreva! Is this command for OME-ZARR, or bdv style OME-ZARR (i.e. does it need an xml file too)?

This command should work for both OME-ZARR and BDV OME-ZARR formats.

K-Meech commented 3 years ago

Ok - great! @KateMoreva @constantinpape Is there a local OME-ZARR file available somewhere I could use? e.g. on the EMBL file share?

KateMoreva commented 3 years ago

Ok - great! @KateMoreva @constantinpape Is there a local OME-ZARR file available somewhere I could use? e.g. on the EMBL file share?

I think g/kreshuk/pape/Work/mobie/covid-if-project/data/20200406_164555_328/images/ome-zarr/nuclei_WellA01_0000.ome.zarr is such an example.

KateMoreva commented 3 years ago

Ok - great! @KateMoreva @constantinpape Is there a local OME-ZARR file available somewhere I could use? e.g. on the EMBL file share?

I think g/kreshuk/pape/Work/mobie/covid-if-project/data/20200406_164555_328/images/ome-zarr/nuclei_WellA01_0000.ome.zarr is such an example.

To open it using Plugins>BigDataViewer>OME ZARR>Open OME ZARR from the file system... please specify the full path to the .ome.zarr file. If you want to use MoBIE instead you will need to use the root path and set the format: new MoBIE( "/g/kreshuk/pape/Work/mobie/covid-if-project", MoBIESettings.settings().imageDataFormat( ImageDataFormat.OmeZarr ) ); (like in the example).

K-Meech commented 3 years ago

@KateMoreva thanks! Unfortunately, I don't have access to the kreshuk lab share. Could you upload it somewhere else? e.g. google drive / own cloud etc if it's not too big. You can email me the link, if that's easier?

constantinpape commented 3 years ago

@K-Meech I will make a copy on the EMCF share.

constantinpape commented 3 years ago

@K-Meech I copied some test data to /g/emcf/pape/ngff/v0.3; there are different axis combinations.

K-Meech commented 3 years ago

Thanks both! I'll have a look through the data :)

K-Meech commented 3 years ago

Ok - so I can write to version 0.2 OME-ZARR now via bioformats2raw, and open it with Open OME ZARR from the file system... There's a super short example on this branch: https://github.com/mobie/mobie-viewer-fiji/blob/writing_ome_zarr/src/test/java/develop/DevelopOmeZarrWriting.java

Some points to solve:

K-Meech commented 3 years ago

@constantinpape I'm still getting to grips with the OME-ZARR format - is the spatial calibration for each axis stored?

constantinpape commented 3 years ago
* it's v0.2 rather than v0.3 - are both versions fully supported in MoBIE?

Yes, they should both be fully supported.

* `bioformats2raw` makes an extra layer of nesting in the file e.g. writing one image to a .zarr would make a subdirectory called `0`,

I think the first 0 is just for the downsampling layers. IMHO this is unfortunate, because it can be easily confused with the chunk hierarchy and it would be better to call it s0. Note that the name here is not specified by the spec but specified in the multiscales metadata. Could you maybe make an issue in bioformats2raw about this? (I.e. ask them to have a different naming scheme for the downscaling)

and then inside that the various downsampling levels 0/1/2 etc... To open this, you have to point to the 0 directory, not to the top .zarr.

In MoBIE it should be possible to open it by pointing to the top-level, because the name of the scaling folders is specified in the metadata. If that does not work could you please make a separate issue about this with a reproducible example.

  • This conversion works by giving the filepath of the image to convert. This is different to the n5 conversion where we rely first on opening in imageJ (i.e. making an ImagePlus) then converting from there. So, we'd need to shuffle a few things around in the project creator. Also, e.g. converting image sequences may be difficult, as I think it needs the image to be a single file.

Maybe you could ask in bioformats2raw if there's a way to go from an opened image?

@constantinpape I'm still getting to grips with the OME-ZARR format - is the spatial calibration for each axis stored?

No it's not stored in there yet, we are working on it...

K-Meech commented 3 years ago

I think the first 0 is just for the downsampling layers.

I think it's more that it's creating a whole dir (0) for the image [I imagine to support the case of converting some file formats that contain multiple images in one file?] This top level's .zattrs does not contain the multiscales metadata, but the one below does. I'll make an issue on bioformats anyway to clear this up + for the naming of the downsampling layers

Maybe you could ask in bioformats2raw if there's a way to go from an opened image?

I'll put another issue on their repo for this, and see if they have suggestions!

K-Meech commented 3 years ago

I made 3 issues covering these points: https://github.com/glencoesoftware/bioformats2raw/issues/109 https://github.com/glencoesoftware/bioformats2raw/issues/108 https://github.com/glencoesoftware/bioformats2raw/issues/110

K-Meech commented 3 years ago

Ok - so all the structure issues we had can be solved with different flags to bioformats2raw. But, there are no options to go directly from an imageplus. (which means everything has to be in one file, i.e. no image sequences etc.)

As the amount of code to convert isn't so big (see https://github.com/glencoesoftware/bioformats2raw/issues/110#issuecomment-887563768 ), I think it'd be worth trying to write something that goes directly from imageplus to ome-zarr. @tischi @constantinpape @KateMoreva - does this exist yet? I see you have some kind of writer here: https://github.com/mobie/bigdataviewer-imageloader/blob/main/src/main/java/de/embl/cba/n5/ome/zarr/writers/N5ZarrWriter.java

tischi commented 3 years ago

I have nothing for writing ZARR, I think the writer just sits there because we copied all the classes related to N5 and ZARR.

to go directly from an imageplus

...go directly from RandomAccessibleInterval would also be fine as ImageJFunctions.wrap could convert between the two.

write something that goes directly from imageplus to ome-zarr, does that exist

I would ask on the forum, although probably the OME people, which are currently on holidays, are probably the only one that know. But maybe they are already back next week? (@constantinpape do you know?)

K-Meech commented 3 years ago

I'll ask in the forum, and see if there are any suggestions :)

K-Meech commented 3 years ago

@constantinpape @KateMoreva I'm starting to put together a draft of an ome-zarr writer. Which version of the ome-zarr readers should I use? The ones in mobie or the ones in bigdataviewer-imageloader?

KateMoreva commented 3 years ago

@constantinpape @KateMoreva I'm starting to put together a draft of an ome-zarr writer. Which version of the ome-zarr readers should I use? The ones in mobie or the ones in bigdataviewer-imageloader?

I think it is better to use mobile reader for now. They would be used as the dependency once we solve the problem with the CI in bigdataviewer-imageloader (hopefully today).

K-Meech commented 3 years ago

@tischi I'm having trouble building the current mobie master - errors on this line: https://github.com/K-Meech/mobie-viewer-fiji/blob/master/src/main/java/de/embl/cba/mobie/bdv/view/AnnotatedIntervalSliceView.java#L59 Are you using a different version of bdv playground?

KateMoreva commented 3 years ago

@tischi I'm having trouble building the current mobie master - errors on this line: https://github.com/K-Meech/mobie-viewer-fiji/blob/master/src/main/java/de/embl/cba/mobie/bdv/view/AnnotatedIntervalSliceView.java#L59 Are you using a different version of bdv playground?

I have the same issue. New show() method implementation: https://github.com/bigdataviewer/bigdataviewer-playground/pull/238/files.

K-Meech commented 3 years ago

@KateMoreva I found a solution for this - you have to build locally the addSourcesWithoutDisplay branch of this repo: https://github.com/bigdataviewer/bigdataviewer-playground/tree/addSourcesWithoutDisplay

tischi commented 3 years ago

Amazing detective work @KateMoreva 😄 ! (ping @K-Meech if you run into the same ⬆️ issue). Yes, sorry, until (hopefully) @NicoKiaru merges this, we need to do this.

K-Meech commented 3 years ago

@constantinpape @KateMoreva Are there any examples of bdv ome-zarr files? With the xml? Thanks!

KateMoreva commented 3 years ago

@constantinpape @KateMoreva Are there any examples of bdv ome-zarr files? With the xml? Thanks!

Bdv.ome.zarr (with XML) example is here: https://s3.embl.de/i2k-2020/project-bdv-ome-zarr.

K-Meech commented 3 years ago

Thanks @KateMoreva ! It errors when I try to open it though.

I'm opening it like: new MoBIE("https://s3.embl.de/i2k-2020/project-bdv-ome-zarr", MoBIESettings.settings().gitProjectBranch( "master" ).imageDataFormat(ImageDataFormat.BdvOmeZarr));

And this is the error:

java.nio.file.InvalidPathException: Illegal char <:> at index 5: https://s3.embl.de/i2k-2020/project-bdv-ome-zarr/Covid19-S4-Area2/images/bdv.ome.zarr/raw.ome.zarr
    at sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
    at sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
    at sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
    at sun.nio.fs.WindowsPath.parse(WindowsPath.java:94)
    at sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:255)
    at java.nio.file.Paths.get(Paths.java:84)
    at de.embl.cba.mobie.MoBIE.openBdvZarrData(MoBIE.java:586)
    at de.embl.cba.mobie.MoBIE.getSourceAndConverter(MoBIE.java:328)
    at de.embl.cba.mobie.MoBIE.lambda$openSourceAndConverters$0(MoBIE.java:199)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)
Exception in thread "main" java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
    at java.util.ArrayList.rangeCheck(ArrayList.java:659)
    at java.util.ArrayList.get(ArrayList.java:435)
    at de.embl.cba.mobie.ui.UserInterfaceHelper.createImageDisplaySettingsPanel(UserInterfaceHelper.java:283)
    at de.embl.cba.mobie.ui.UserInterface.createDisplaySettingPanel(UserInterface.java:100)
    at de.embl.cba.mobie.ui.UserInterface.addSourceDisplay(UserInterface.java:92)
    at de.embl.cba.mobie.view.ViewManager.showSourceDisplay(ViewManager.java:256)
    at de.embl.cba.mobie.view.ViewManager.show(ViewManager.java:215)
    at de.embl.cba.mobie.MoBIE.openDataset(MoBIE.java:220)
    at de.embl.cba.mobie.MoBIE.openDataset(MoBIE.java:156)
    at de.embl.cba.mobie.MoBIE.<init>(MoBIE.java:92)
    at projects.OpenRemoteBdvOmeZarr.main(OpenRemoteBdvOmeZarr.java:14)
constantinpape commented 3 years ago

new MoBIE("https://s3.embl.de/i2k-2020/project-bdv-ome-zarr", MoBIESettings.settings().gitProjectBranch( "master" ).imageDataFormat(ImageDataFormat.BdvOmeZarr));

I think you need to use ImageDataForma.BdvOmeZarrS3 to load from s3.

K-Meech commented 3 years ago

That was it - thanks @constantinpape !

Another point - I'm modifying the zarr writer at the moment, and I'm wondering how is best to deal with multiple channels or timepoints? i.e. would you ever want a chunksize larger than 1 for these axes? And if so, is this supported by bigdataviewer?

The current ZarrWriter implementation that I'm modifying (https://github.com/mobie/mobie-viewer-fiji/blob/master/src/main/java/de/embl/cba/mobie/n5/zarr/N5ZarrWriter.java), only chunks in zyx, so it would be easiest to enforce a chunksize of 1 for the other axes.

constantinpape commented 3 years ago

i.e. would you ever want a chunksize larger than 1 for these axes?

Indeed, I think chunksize bigger than 1 doesn't make much sense for our use-cases.

And if so, is this supported by bigdataviewer?

I think that it would work, but would not be very efficient, since bdv only ever accesses a single channel or timepoint and multiple would need to be loaded with chunk-size > 1.

K-Meech commented 3 years ago

Ok great! I'll restrict to a chunksize of 1 for time/channels.

K-Meech commented 3 years ago

Alright - I have a working draft of the ome.zarr writer!! I need to test/tidy up a few parts early next week, but should be a PR soon :)

So @tischi @constantinpape @KateMoreva - if you're planning to move a bunch of stuff from the mobie-viewer-fiji repo to the bigdataviewer-imageloader repo, maybe wait until this is merged too.

KateMoreva commented 3 years ago

Alright - I have a working draft of the ome.zarr writer!! I need to test/tidy up a few parts early next week, but should be a PR soon :)

So @tischi @constantinpape @KateMoreva - if you're planning to move a bunch of stuff from the mobie-viewer-fiji repo to the bigdataviewer-imageloader repo, maybe wait until this is merged too.

Ok, thank you!

P. S. MoBIE version with the dependency on the bigdataviewer-imageloader repo is here https://github.com/mobie/mobie-viewer-fiji/pull/405.

K-Meech commented 3 years ago

@constantinpape @KateMoreva Could you send me an example xml file from: https://s3.embl.de/i2k-2020/project-bdv-ome-zarr ? I'm trying to match the xml format for writing BdvOmeZarr. Thanks!

constantinpape commented 3 years ago

@K-Meech you can download the whole project here: https://oc.embl.de/index.php/s/KLTaSgX1c4zgnHP

K-Meech commented 3 years ago

Perfect - thanks!

K-Meech commented 3 years ago

PR here: https://github.com/mobie/mobie-viewer-fiji/pull/408

constantinpape commented 3 years ago

408 is merged, so I think we can close this one.