Add optional OMERO rendering metadata #160

melissalinkert · 2022-08-09T23:38:39Z

This includes introducing min/max calculation so that the channel min/max
values are written in each group. See https://ngff.openmicroscopy.org/latest/#omero-md

The --no-omero option turns off OMERO metadata population, which also turns
off min/max calculation.

I wouldn't expect this to be merged as-is, it's just a place to start. A few things that definitely need other opinions:

should OMERO rendering metadata be on by default (better for NGFF imports into OMERO), or off by default (faster for everyone else, consistent with existing behavior)?
which channels should have active set? is this user-configurable?
what should the OMERO id be? it's 0 here, but should it be omitted or user-configurable instead?
do we want to replicate all of the color picking logic that OMERO uses, or just use whatever Color was set on each Channel?
what is the difference between start/end and min/max in the window?

Once we're happy with how this works, I'll add tests before merging.

This includes introducing min/max calculation so that the channel min/max values are written in each group. See https://ngff.openmicroscopy.org/latest/#omero-md The --no-omero option turns off OMERO metadata population, which also turns off min/max calculation.

melissalinkert · 2022-08-18T17:57:16Z

a9e555e and ec67911 should fix color and min/max issues mentioned separately by @erindiel.

8ab3ceb adds color picking logic based on https://github.com/ome/omero-renderer/blob/master/src/main/java/omeis/providers/re/ColorsFactory.java.

chris-allan · 2022-08-24T11:08:04Z

In my opinion if we want this to be usable it has to mirror what OMERO would do for default rendering settings exactly. There's probably a discussion with the wider OME-NGFF specification team as to what attributes are explicitly required and which ones are optional but I would drop the following root attributes:

id
version

Dropping name is debatable given the presence of OME-XML metadata but there's probably no harm in including it in what bioformats2raw writes out.

channels loosely represents the fields of ChannelBinding and rdefs of RenderingDef. You can see how these fields are mapped from the OMERO data model here:

https://github.com/ome/omero-web/blob/master/omeroweb/webgateway/marshal.py#L107

The "rules" for how the default rendering settings are populated are loosely defined and have remained pretty static for over 10 years. They are located in RenderingSettingsImpl within the omero-server component. The most important methods are resetDefaults() and resetChannelBindings():

It's going to be very hard to bring this code in without pulling in the OMERO server dependency tree so I would propose we continue with the strategy you've started @melissalinkert: replicate the decisions here in bioformats2raw and add test cases for them.

...and add more tests.

melissalinkert · 2022-08-29T21:05:40Z

I think 6693198 addresses all of the open comments here, but if any remaining discrepancies turn up during review they should be easy enough to fix.

melissalinkert · 2022-08-29T21:10:25Z

In terms of spec changes, we'll likely want to keep an eye on ome/ngff#78 and update in a future PR once that's finalized.

sbesson · 2022-09-06T14:45:34Z

what is the difference between start/end and min/max in the window?

The answer might be partial but at least based on the implementation in the OMERO.py gateway:

start/end reflects the pixel range to be used when rendering the data
min/max reflects either the computed global minimal/maximal values or in the absence of such data, the maximum pixel range as given by the pixel type

Taking this image which was exported as OME-NGFF (https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.4/idr0062A/6001240.zarr) as an example

(base) sbesson@Sebastiens-MacBook-Pro-2 ~ % curl https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.4/idr0062A/6001240.zarr/.zattrs
...
    "omero": {
        "channels": [
            {
                "active": true,
                "coefficient": 1.0,
                "color": "0000FF",
                "family": "linear",
                "inverted": false,
                "label": "LaminB1",
                "window": {
                    "end": 1500.0,
                    "max": 65535.0,
                    "min": 0.0,
                    "start": 0.0
                }
            },
            {
                "active": true,
                "coefficient": 1.0,
                "color": "FFFF00",
                "family": "linear",
                "inverted": false,
                "label": "Dapi",
                "window": {
                    "end": 1500.0,
                    "max": 65535.0,
                    "min": 0.0,
                    "start": 0.0
                }
            }
        ],
        "id": 1,
        "rdefs": {
            "defaultT": 0,
            "defaultZ": 118,
            "model": "color"
        },
        "version": "0.4"
    }
}%

sbesson

One suggestion to store the name at the multiscales level

should OMERO rendering metadata be on by default (better for NGFF imports into OMERO), or off by default (faster for everyone else, consistent with existing behavior)?

Where does the performance hit come from? Assuming we manage to get similar performance metrics, my opinion is that storing this additional metadata in the converted output should become the default. Consumers can then choose whether they use it or not

what should the OMERO id be? it's 0 here, but should it be omitted or user-configurable instead?

I also support not writing omero.id. In an OMERO export workflow, this can be used to capture some provenance metadata with the ID of the source object ID but that does not apply here.
If some form of identifier was valuable, we might want to look into the uuid proposal

src/main/java/com/glencoesoftware/bioformats2raw/Converter.java

melissalinkert · 2022-09-09T15:44:53Z

The expected performance hit comes from min/max calculation. While we're reading every tile no matter what, actually unpacking each pixel only happens if min/max calculation is performed, which is only when OMERO metadata is being set. I haven't done any concrete benchmarking there yet, but would be surprised if there isn't a noticeable difference.

joshmoore

One minor naming thought

src/main/java/com/glencoesoftware/bioformats2raw/Converter.java

sbesson

Tested the execution of time ./bin/bioformats2raw --omero vs time ./bin/bioformats2raw --no-omero for a few modalities from the HEAD of this branch.

Using a sample BBBC017 plate:

./bioformats2raw-0.5.0-SNAPSHOT/bin/bioformats2raw --omero   1922.56s user 2022.15s system 152% cpu 43:09.93 total
./bioformats2raw-0.5.0-SNAPSHOT/bin/bioformats2raw --no-omero    1256.84s user 1576.65s system 195% cpu 24:12.64 total

and the diff between

(base) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/NIRHTa+001.ome.zarr/.zattrs no_omero/NIRHTa+001.ome.zarr/.zattrs 
(base) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/NIRHTa+001.ome.zarr/A/1/.zattrs no_omero/NIRHTa+001.ome.zarr/A/1/.zattrs 
(base) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/NIRHTa+001.ome.zarr/A/1/0/.zattrs no_omero/NIRHTa+001.ome.zarr/A/1/0/.zattrs
38,85c38
<   } ],
<   "omero" : {
<     "channels" : [ {
<       "color" : "FF0000",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : null,
<       "window" : {
<         "min" : "Infinity",
<         "max" : "-Infinity",
<         "start" : "Infinity",
<         "end" : "-Infinity"
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "00FF00",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : null,
<       "window" : {
<         "min" : "Infinity",
<         "max" : "-Infinity",
<         "start" : "Infinity",
<         "end" : "-Infinity"
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "0000FF",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : null,
<       "window" : {
<         "min" : 154.0,
<         "max" : 3396.0,
<         "start" : 154.0,
<         "end" : 3396.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     } ],
<     "rdefs" : {
<       "defaultT" : 0,
<       "model" : "color",
<       "defaultZ" : 0
<     }
<   }
---
>   } ]

using LuCa-7color_Scan1.ome.tiff

./bioformats2raw-0.5.0-SNAPSHOT/bin/bioformats2raw --omero    194.89s user 15.59s system 405% cpu 51.870 total
./bioformats2raw-0.5.0-SNAPSHOT/bin/bioformats2raw --no-omero    149.65s user 15.37s system 409% cpu 40.306 total

and

(base) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/LuCa-7color_Scan1.ome.zarr/.zattrs no_omero/LuCa-7color_Scan1.ome.zarr/.zattrs 
(base) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/LuCa-7color_Scan1.ome.zarr/0/.zattrs no_omero/LuCa-7color_Scan1.ome.zarr/0/.zattrs 
82,155c82
<   } ],
<   "omero" : {
<     "channels" : [ {
<       "color" : "0000FF",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "DAPI",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "00FF00",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "FITC",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "FFFF00",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "CY3",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "FF8000",
<       "coefficient" : 1,
<       "active" : false,
<       "label" : "Texas Red",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "FF0000",
<       "coefficient" : 1,
<       "active" : false,
<       "label" : "CY5",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     } ],
<     "rdefs" : {
<       "defaultT" : 0,
<       "model" : "color",
<       "defaultZ" : 0
<     }
<   }
---
>   } ]

using Leica-2.ome.tiff

./bin/bioformats2raw --omero    559.48s user 151.70s system 395% cpu 2:59.84 total
./bin/bioformats2raw --no-omero    475.75s user 147.99s system 402% cpu 2:34.92 total

and

(base) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/Leica-2.ome.zarr/.zattrs no_omero/Leica-2.ome.zarr/.zattrs 
(base) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/Leica-2.ome.zarr/0/.zattrs no_omero/Leica-2.ome.zarr/0/.zattrs 
64,111c64
<   } ],
<   "omero" : {
<     "channels" : [ {
<       "color" : "FF0000",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : null,
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "00FF00",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : null,
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "0000FF",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : null,
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     } ],
<     "rdefs" : {
<       "defaultT" : 0,
<       "model" : "color",
<       "defaultZ" : 0
<     }
<   }
---
>   } ]

Based on these measurements, the additional overhead due to the bytes unpacking can add an extra 50% on the total conversion time.

In terms of metadata, we should address the writing of "label" : null and infinite rendering window. I assume the former is simply an extra check of the channel name. I am unsure of where the infinite values come from.

melissalinkert · 2022-09-12T16:30:30Z

I think getting rid of "label": null means implementing the channel name logic from https://github.com/ome/omero-blitz/blob/5d27e4771af5c4c457464090edb5b450d1c11e1d/src/main/java/ome/formats/model/ChannelProcessor.java. That's easy enough to add here.

@kkoz mentioned seeing Infinity/-Infinity for the min/max sometimes too, so I'll need to investigate that further. That implies that no min/max calculation results are available, which is probably some bug in aggregating results across all workers.

See https://github.com/ome/omero-blitz/blob/5d27e4771af5c4c457464090edb5b450d1c11e1d/src/main/java/ome/formats/model/ChannelProcessor.java

The "known" values from MinMaxCalculator need to be used instead of the "global" values. The "global" values will be null if a MinMaxCalculator instance did not read every plane in the current series. This is pretty much guaranteed to be the case when there are multiple workers.

sbesson

Re-executed the tests described in #160 (review). The conversion time and the overhead due to the min/max computation was comparable in all cases except for the plate sample where the conversion time is largely identical with/without min/max calculation 👍

(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated %  time bioformats2raw --omero ome-tiff/BBBC017/single-file/NIRHTa+001.ome.tiff ome-ngff/omero/NIRHTa+001.ome.zarr
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.esotericsoftware.kryo.util.UnsafeUtil (file:/Users/sbesson/Documents/GitHub/bioformats2raw/bioformats2raw-0.5.0-SNAPSHOT/lib/kryo-4.0.2.jar) to constructor java.nio.DirectByteBuffer(long,int,java.lang.Object)
WARNING: Please consider reporting this to the maintainers of com.esotericsoftware.kryo.util.UnsafeUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
bioformats2raw --omero ome-tiff/BBBC017/single-file/NIRHTa+001.ome.tiff   1289.86s user 1574.75s system 200% cpu 23:51.66 total
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated %  time bioformats2raw --no-omero  ome-tiff/BBBC017/single-file/NIRHTa+001.ome.tiff ome-ngff/no_omero/NIRHTa+001.ome.zarr
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.esotericsoftware.kryo.util.UnsafeUtil (file:/Users/sbesson/Documents/GitHub/bioformats2raw/bioformats2raw-0.5.0-SNAPSHOT/lib/kryo-4.0.2.jar) to constructor java.nio.DirectByteBuffer(long,int,java.lang.Object)
WARNING: Please consider reporting this to the maintainers of com.esotericsoftware.kryo.util.UnsafeUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
bioformats2raw --no-omero ome-tiff/BBBC017/single-file/NIRHTa+001.ome.tiff   1240.77s user 1569.51s system 192% cpu 24:17.87 total


(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated % time bioformats2raw --omero ome-tiff/public/2016-06/sub-resolutions/Brightfield/Leica-2/Leica-2.ome.tiff ome-ngff/omero/Leica-2.ome.zarr
bioformats2raw --omero  ome-ngff/omero/Leica-2.ome.zarr  629.34s user 171.40s system 397% cpu 3:21.46 total
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated % time bioformats2raw --no-omero ome-tiff/public/2016-06/sub-resolutions/Brightfield/Leica-2/Leica-2.ome.tiff ome-ngff/no_omero/Leica-2.ome.zarr
bioformats2raw --no-omero  ome-ngff/no_omero/Leica-2.ome.zarr  534.91s user 178.80s system 399% cpu 2:58.43 total

(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated % time bioformats2raw --omero ome-tiff/public/2016-06/sub-resolutions/Fluorescence/LuCa-7color_Scan1.ome.tiff ome-ngff/omero/LuCa-7color_Scan1.ome.zarr
bioformats2raw --omero  ome-ngff/omero/LuCa-7color_Scan1.ome.zarr  202.83s user 18.76s system 402% cpu 55.049 total
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated % time bioformats2raw --no-omero ome-tiff/public/2016-06/sub-resolutions/Fluorescence/LuCa-7color_Scan1.ome.tiff ome-ngff/no_omero/LuCa-7color_Scan1.ome.zarr
bioformats2raw --no-omero  ome-ngff/no_omero/LuCa-7color_Scan1.ome.zarr  153.94s user 16.39s system 411% cpu 41.394 total
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated % rm -rf  ome-ngff/no_omero/NIRHTa+001.ome.zarr

Looking at the metadata differences, there is no longer null of +/- Infinity values and the new rendering window all look sensible. The label field are set to Channel xxx by default which is the expectation of the population code:

(java_dev) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/NIRHTa+001.ome.zarr/.zattrs no_omero/NIRHTa+001.ome.zarr/.zattrs 
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/NIRHTa+001.ome.zarr/A/1/.zattrs no_omero/NIRHTa+001.ome.zarr/A/1/.zattrs 
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/NIRHTa+001.ome.zarr/A/1/0/.zattrs no_omero/NIRHTa+001.ome.zarr/A/1/0/.zattrs 
38,85c38
<   } ],
<   "omero" : {
<     "channels" : [ {
<       "color" : "FF0000",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "Channel 0",
<       "window" : {
<         "min" : 163.0,
<         "max" : 4095.0,
<         "start" : 163.0,
<         "end" : 4095.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "00FF00",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "Channel 1",
<       "window" : {
<         "min" : 104.0,
<         "max" : 3563.0,
<         "start" : 104.0,
<         "end" : 3563.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "0000FF",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "Channel 2",
<       "window" : {
<         "min" : 154.0,
<         "max" : 3396.0,
<         "start" : 154.0,
<         "end" : 3396.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     } ],
<     "rdefs" : {
<       "defaultT" : 0,
<       "model" : "color",
<       "defaultZ" : 0
<     }
<   }
---
>   } ]
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % 
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % ls
no_omero	omero
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % dif
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/Leica-2.ome.zarr/0/.zattrs no_omero/Leica-2.ome.zarr/0/.zattrs 
64,111c64
<   } ],
<   "omero" : {
<     "channels" : [ {
<       "color" : "FF0000",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "Channel 0",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "00FF00",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "Channel 1",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "0000FF",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "Channel 2",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     } ],
<     "rdefs" : {
<       "defaultT" : 0,
<       "model" : "color",
<       "defaultZ" : 0
<     }
<   }
---
>   } ]
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/LuCa-7color_Scan1.ome.zarr/.zattrs no_omero/LuCa-7color_Scan1.ome.zarr/.zattrs 
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff omero/LuCa-7color_Scan1.ome.zarr/0/.zattrs no_omero/LuCa-7color_Scan1.ome.zarr/0/.zattrs 
82,155c82
<   } ],
<   "omero" : {
<     "channels" : [ {
<       "color" : "0000FF",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "DAPI",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "00FF00",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "FITC",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "FFFF00",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "CY3",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "FF8000",
<       "coefficient" : 1,
<       "active" : false,
<       "label" : "Texas Red",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "FF0000",
<       "coefficient" : 1,
<       "active" : false,
<       "label" : "CY5",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     } ],
<     "rdefs" : {
<       "defaultT" : 0,
<       "model" : "color",
<       "defaultZ" : 0
<     }
<   }
---
>   } ]

The only outstanding issue is the fact channel names do not seem to be generated from the emission wavelength in my environment - see inline comment about test failure

sbesson · 2022-09-13T14:54:16Z

src/main/java/com/glencoesoftware/bioformats2raw/Colors.java

+   * @param c channel index
+   * @return channel color
+   */
+  public static Color getColor(OMEXMLMetadata meta, int series, int c) {


Outside the scope of this PR, assuming this is the same logic as the one implemented in the server components, it would make sense to capture the migration of this logic to Bio-Formats e.g. as a FormatTools helper or similar. If a new Bio-Formats release included this backwards-compatible API addition, downstream components could be updated to consume it.

src/test/java/com/glencoesoftware/bioformats2raw/test/ZarrTest.java

…r linkages

sbesson · 2022-09-13T16:29:36Z

e444287 fixes the test failure I was encountering previously. I will carry out a final round of testing of this PR tomorrow morning.

sbesson

(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated % time bioformats2raw --no-minmax ome-tiff/public/2016-06/sub-resolutions/Brightfield/Leica-2/Leica-2.ome.tiff ome-ngff/no_minmax/Leica-2.ome.zarr
bioformats2raw --no-minmax  ome-ngff/no_minmax/Leica-2.ome.zarr  478.98s user 165.74s system 399% cpu 2:41.25 total
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated % time bioformats2raw --minmax ome-tiff/public/2016-06/sub-resolutions/Brightfield/Leica-2/Leica-2.ome.tiff ome-ngff/minmax/Leica-2.ome.zarr
bioformats2raw --minmax  ome-ngff/minmax/Leica-2.ome.zarr  580.50s user 153.07s system 402% cpu 3:02.11 total

(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated % time bioformats2raw --no-minmax ome-tiff/public/2016-06/sub-resolutions/Fluorescence/LuCa-7color_Scan1.ome.tiff ome-ngff/no_minmax/LuCa-7color_Scan1.ome.zarr
bioformats2raw --no-minmax  ome-ngff/no_minmax/LuCa-7color_Scan1.ome.zarr  158.91s user 16.94s system 397% cpu 44.194 total
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated % time bioformats2raw --minmax ome-tiff/public/2016-06/sub-resolutions/Fluorescence/LuCa-7color_Scan1.ome.tiff ome-ngff/minmax/LuCa-7color_Scan1.ome.zarr
bioformats2raw --minmax  ome-ngff/minmax/LuCa-7color_Scan1.ome.zarr  188.75s user 14.79s system 408% cpu 49.829 total

(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated % time bioformats2raw --no-minmax ome-tiff/BBBC017/single-file/NIRHTa+001.ome.tiff ome-ngff/no_minmax/NIRHTa+001.ome.zar
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.esotericsoftware.kryo.util.UnsafeUtil (file:/Users/sbesson/Documents/GitHub/bioformats2raw/bioformats2raw-0.5.0-SNAPSHOT/lib/kryo-4.0.2.jar) to constructor java.nio.DirectByteBuffer(long,int,java.lang.Object)
WARNING: Please consider reporting this to the maintainers of com.esotericsoftware.kryo.util.UnsafeUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
bioformats2raw --no-minmax ome-tiff/BBBC017/single-file/NIRHTa+001.ome.tiff   1245.54s user 1524.63s system 195% cpu 23:34.38 total
(java_dev) sbesson@Sebastiens-MacBook-Pro-2 curated % time bioformats2raw --minmax ome-tiff/BBBC017/single-file/NIRHTa+001.ome.tiff ome-ngff/minmax/NIRHTa+001.ome.zar
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.esotericsoftware.kryo.util.UnsafeUtil (file:/Users/sbesson/Documents/GitHub/bioformats2raw/bioformats2raw-0.5.0-SNAPSHOT/lib/kryo-4.0.2.jar) to constructor java.nio.DirectByteBuffer(long,int,java.lang.Object)
WARNING: Please consider reporting this to the maintainers of com.esotericsoftware.kryo.util.UnsafeUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
bioformats2raw --minmax ome-tiff/BBBC017/single-file/NIRHTa+001.ome.tiff   1245.07s user 1488.08s system 196% cpu 23:12.90 total

and

(aws) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff minmax/NIRHTa+001.ome.zar/.zattrs no_minmax/NIRHTa+001.ome.zar/.zattrs 
(aws) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff minmax/NIRHTa+001.ome.zar/A/1/.zattrs no_minmax/NIRHTa+001.ome.zar/A/1/.zattrs 
(aws) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff  minmax/NIRHTa+001.ome.zar/A/1/0/.zattrs no_minmax/NIRHTa+001.ome.zar/A/1/0/.zattrs 
38,85c38
<   } ],
<   "omero" : {
<     "channels" : [ {
<       "color" : "FF0000",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "Channel 0",
<       "window" : {
<         "min" : 163.0,
<         "max" : 4095.0,
<         "start" : 163.0,
<         "end" : 4095.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "00FF00",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "Channel 1",
<       "window" : {
<         "min" : 104.0,
<         "max" : 3563.0,
<         "start" : 104.0,
<         "end" : 3563.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "0000FF",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "Channel 2",
<       "window" : {
<         "min" : 154.0,
<         "max" : 3396.0,
<         "start" : 154.0,
<         "end" : 3396.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     } ],
<     "rdefs" : {
<       "defaultT" : 0,
<       "model" : "color",
<       "defaultZ" : 0
<     }
<   }
---
>   } ]
(aws) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff minmax/LuCa-7color_Scan1.ome.zarr/.zattrs no_minmax/LuCa-7color_Scan1.ome.zarr/.zattrs 
(aws) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff minmax/LuCa-7color_Scan1.ome.zarr/0/.zattrs no_minmax/LuCa-7color_Scan1.ome.zarr/0/.zattrs 
82,155c82
<   } ],
<   "omero" : {
<     "channels" : [ {
<       "color" : "0000FF",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "DAPI",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "00FF00",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "FITC",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "FFFF00",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "CY3",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "FF8000",
<       "coefficient" : 1,
<       "active" : false,
<       "label" : "Texas Red",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "FF0000",
<       "coefficient" : 1,
<       "active" : false,
<       "label" : "CY5",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     } ],
<     "rdefs" : {
<       "defaultT" : 0,
<       "model" : "color",
<       "defaultZ" : 0
<     }
<   }
---
>   } ]
(aws) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff minmax/Leica-2.ome.zarr/.zattrs no_minmax/Leica-2.ome.zarr/.zattrs 
(aws) sbesson@Sebastiens-MacBook-Pro-2 ome-ngff % diff minmax/Leica-2.ome.zarr/0/.zattrs no_minmax/Leica-2.ome.zarr/0/.zattrs 
64,111c64
<   } ],
<   "omero" : {
<     "channels" : [ {
<       "color" : "FF0000",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "Channel 0",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "00FF00",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "Channel 1",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     }, {
<       "color" : "0000FF",
<       "coefficient" : 1,
<       "active" : true,
<       "label" : "Channel 2",
<       "window" : {
<         "min" : 0.0,
<         "max" : 255.0,
<         "start" : 0.0,
<         "end" : 255.0
<       },
<       "family" : "linear",
<       "inverted" : false
<     } ],
<     "rdefs" : {
<       "defaultT" : 0,
<       "model" : "color",
<       "defaultZ" : 0
<     }
<   }
---
>   } ]

Overall:

the new unit tests are now reliably passing
👍 for allowing a codepath allowing to compute this metadata and store it in the most appropriate location as defined by the current OME-NGFF spec. Obviously, this might need to be revisited as the rendering specification evolves - see the draft proposal
the min/max computation overhead now feels very reasonable so having the min/max as the default should be acceptable
the code also ports some of the channel name population logic from OMERO when the metadata is missing from the Channel.Name

The only notable difference with the OMERO channel name code is that this logic falls back to using Channel <index> as the default while in OMERO, RGB images currently are labelled as 0, 1, 2. I am happy to see this merged in its current form but it would make sense to review the impact of this particular decision in the context of a full conversion+import pipeline.

sbesson · 2022-09-21T16:15:33Z

In addition to the metrics above which are reporting computation time, I minimally tested that the metadata written by the default --minmax option can be consumed by a client aware of the omero metadata specification.
Using napari with the napari-ome-zarr plugin installed

When using an OME-NGFF dataset created with --no-minmax, the channel names are derived from the image names and default colors are assigned

When using an OME-NGFF dataset created with --minmax, the channel are correctly named and the color match the expectation

For reference, this is the viewport of the source OME-TIFF image imported into OMERO with the default settings:

See glencoesoftware/bioformats2raw#160

joshmoore · 2022-09-26T13:34:22Z

Note: decision was made at the Formats meeting today to additionally add "transitional" to the "omero" metadata section of the NGFF spec.

…to omero-metadata

melissalinkert requested review from kkoz and erindiel August 9, 2022 23:38

melissalinkert mentioned this pull request Aug 9, 2022

Fix formatting of link to WebGateway documentation ome/ngff#134

Merged

melissalinkert added 3 commits August 16, 2022 20:12

Fix color formatting

a9e555e

Aggregate min/max values from all readers

ec67911

Add color picking logic and unit tests

8ab3ceb

Fix a few small issues with OMERO rendering metadata

6693198

...and add more tests.

sbesson requested changes Sep 9, 2022

View reviewed changes

src/main/java/com/glencoesoftware/bioformats2raw/Converter.java Outdated Show resolved Hide resolved

Move name from omero to multiscale

1729af5

sbesson self-requested a review September 9, 2022 16:33

joshmoore reviewed Sep 12, 2022

View reviewed changes

src/main/java/com/glencoesoftware/bioformats2raw/Converter.java Outdated Show resolved Hide resolved

sbesson requested changes Sep 12, 2022

View reviewed changes

melissalinkert added 2 commits September 12, 2022 18:37

Update channel names to mirror logic in OMERO's ChannelProcessor

515ae8a

See https://github.com/ome/omero-blitz/blob/5d27e4771af5c4c457464090edb5b450d1c11e1d/src/main/java/ome/formats/model/ChannelProcessor.java

melissalinkert requested a review from sbesson September 12, 2022 23:41

Change --no-omero option to --no-minmax

ad227de

sbesson reviewed Sep 13, 2022

View reviewed changes

Make sure MetadataStore references are resolved before checking filte…

e444287

…r linkages

sbesson self-requested a review September 13, 2022 16:29

sbesson approved these changes Sep 15, 2022

View reviewed changes

melissalinkert added a commit to melissalinkert/raw2ometiff-1 that referenced this pull request Sep 21, 2022

Preserve OMERO rendering metadata

4c98392

See glencoesoftware/bioformats2raw#160

melissalinkert mentioned this pull request Sep 21, 2022

Preserve OMERO rendering metadata glencoesoftware/raw2ometiff#82

Closed

Merge branch 'master' of github.com:glencoesoftware/bioformats2raw in…

41be21d

…to omero-metadata

chris-allan merged commit 34de2c3 into glencoesoftware:master Sep 28, 2022

sbesson mentioned this pull request Oct 31, 2022

Color metadata & single-channel use case #170

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add optional OMERO rendering metadata #160

Add optional OMERO rendering metadata #160

melissalinkert commented Aug 9, 2022

melissalinkert commented Aug 18, 2022

chris-allan commented Aug 24, 2022

melissalinkert commented Aug 29, 2022

melissalinkert commented Aug 29, 2022

sbesson commented Sep 6, 2022

sbesson left a comment

melissalinkert commented Sep 9, 2022

joshmoore left a comment

sbesson left a comment

melissalinkert commented Sep 12, 2022

sbesson left a comment

sbesson Sep 13, 2022 •

edited

Loading

sbesson commented Sep 13, 2022

sbesson left a comment

sbesson commented Sep 21, 2022

joshmoore commented Sep 26, 2022

Add optional OMERO rendering metadata #160

Add optional OMERO rendering metadata #160

Conversation

melissalinkert commented Aug 9, 2022

melissalinkert commented Aug 18, 2022

chris-allan commented Aug 24, 2022

melissalinkert commented Aug 29, 2022

melissalinkert commented Aug 29, 2022

sbesson commented Sep 6, 2022

sbesson left a comment

Choose a reason for hiding this comment

melissalinkert commented Sep 9, 2022

joshmoore left a comment

Choose a reason for hiding this comment

sbesson left a comment

Choose a reason for hiding this comment

melissalinkert commented Sep 12, 2022

sbesson left a comment

Choose a reason for hiding this comment

sbesson Sep 13, 2022 • edited Loading

Choose a reason for hiding this comment

sbesson commented Sep 13, 2022

sbesson left a comment

Choose a reason for hiding this comment

sbesson commented Sep 21, 2022

joshmoore commented Sep 26, 2022

sbesson Sep 13, 2022 •

edited

Loading