Add support for ome-ngff-v0.4

[constantinpape]

See discussions in
ome/ngff#57
and example data at
https://s3.embl.de/i2k-2020/ngff-example-data/v0.4

[constantinpape]

Observation from @KateMoreva the version in the example files is still 0.3, I need to fix this.

[constantinpape]

@K-Meech will look into writing the new version.
For reading we still want to support 0.3; for writing not so important.
We can sort of drop the ome.zarr.xml with this addition.

[constantinpape]

For validating: json validation: ome/ngff#69 (comment)
ome-zarr-py: https://github.com/ome/ome-zarr-py

[constantinpape]

@KateMoreva I checked the version of the example files I shared (https://s3.embl.de/i2k-2020/ngff-example-data/v0.4), but I think they have the correct version:

$ wget https://s3.embl.de/i2k-2020/ngff-example-data/v0.4/cyx.ome.zarr/.zattrs -O cyx-attrs
$ cat cyx-attrs | grep version

            "version": "0.4"

Could you please double check this and describe the issue with the version number more concretely if you still have it?

[K-Meech]

@constantinpape Do you also have these files locally somewhere on the group shares?

[KateMoreva]

@KateMoreva I checked the version of the example files I shared (https://s3.embl.de/i2k-2020/ngff-example-data/v0.4), but I think they have the correct version:

$ wget https://s3.embl.de/i2k-2020/ngff-example-data/v0.4/cyx.ome.zarr/.zattrs -O cyx-attrs
$ cat cyx-attrs | grep version

            "version": "0.4"

Could you please double check this and describe the issue with the version number more concretely if you still have it?

Oh, yes, the example files have v 0.4, I meant the PR file: https://github.com/ome/ngff/pull/57/files#diff-ffe6148e5d9f47acc4337bb319ed4503a810214933e51f5f3e46a227b10e3fcdL241
Sorry for the confusion :(

[constantinpape]

@KateMoreva

Oh, yes, the example files have v 0.4, I meant the PR file: https://github.com/ome/ngff/pull/57/files#diff-ffe6148e5d9f47acc4337bb319ed4503a810214933e51f5f3e46a227b10e3fcdL241

Oh, I see. We are working on some mechanism to bump all the version strings automatically, so that's why it's still at 0.3 there. Just ignore that, but Indeed it can be confusing.

@K-Meech

@constantinpape Do you also have these files locally somewhere on the group shares?

/g/kreshuk/pape/Work/mobie/ngff/ome-ngff-prototypes/single_image/v0.4
Let me know if you don't have access to it, then I can copy it somewhere else.

[KateMoreva]

/g/kreshuk/pape/Work/mobie/ngff/ome-ngff-prototypes/single_image/v0.4 Let me know if you don't have access to it, then I can copy it somewhere else.

Thank you for this info! It will be useful for file system reading.

[K-Meech]

So I'm starting to look into updating the writers for v0.4 ome-zarr. @tischi @constantinpape Do we want to keep the ability to write 0.3? Or just shift everything to 0.4?

[tischi]

I guess as long as we can still read 0.3, I would be fine with only writing to the latest version (that we can read).

[constantinpape]

So I'm starting to look into updating the writers for v0.4 ome-zarr. @tischi @constantinpape Do we want to keep the ability to write 0.3? Or just shift everything to 0.4?

I agree; it's enough if we write the latest version (i.e. 0.4 now), but we need to keep support for reading older versions.
Just to note: there are still some small changes for 0.4 pending and also reading does not fully work yet. I will continue working on this now and will ping you once there are updates.

[tischi]

@constantinpape @KateMoreva @K-Meech
Where in our code do we check the OME.Zarr version when reading?
Using the current develop branch I am getting an error reading older zarr versions. The error is here:

        zarrAxes = n5 instanceof N5OmeZarrReader ? ((N5OmeZarrReader) n5).getAxes() :
                n5 instanceof N5S3OmeZarrReader ? ((N5S3OmeZarrReader) n5).getAxes() : ZarrAxes.NOT_SPECIFIED;

        long nC = 1;
        if (attributes.getNumDimensions() > 4) {
            nC = attributes.getDimensions()[C];
        }
        if (zarrAxes.is4DWithChannels()) {
            nC = attributes.getDimensions()[C];
        }

...because zarrAxes is null...

Maybe we cannot read old OME.zarr versions at the moment?

[KateMoreva]

@constantinpape @KateMoreva @K-Meech Where in our code do we check the OME.Zarr version when reading? Using the current develop branch I am getting an error reading older zarr versions. The error is here:

        zarrAxes = n5 instanceof N5OmeZarrReader ? ((N5OmeZarrReader) n5).getAxes() :
                n5 instanceof N5S3OmeZarrReader ? ((N5S3OmeZarrReader) n5).getAxes() : ZarrAxes.NOT_SPECIFIED;

        long nC = 1;
        if (attributes.getNumDimensions() > 4) {
            nC = attributes.getDimensions()[C];
        }
        if (zarrAxes.is4DWithChannels()) {
            nC = attributes.getDimensions()[C];
        }

...because zarrAxes is null...

Maybe we cannot read old OME.zarr versions at the moment?

We can read v3, I'll fix the older variants now.

[tischi]

Hm, I I think I am getting the error reading from: "version": "0.3.0"

Examples:

-bash-4.2$ cat ./Sample3_Rafael_LM_crop_5_Trans.ome.zarr/.zattrs 
{
  "multiscales": [
    {
      "axes": [
        "z",
        "y",
        "x"
      ],
      "datasets": [
        {
          "path": "s0"
        },
        {
          "path": "s1"
        },
        {
          "path": "s2"
        },
        {
          "path": "s3"
        },
        {
          "path": "s4"
        },
        {
          "path": "s5"
        },
        {
          "path": "s6"
        }
      ],
      "name": "Sample3_Rafael_LM_crop_5_Trans",
      "type": "Average",
      "version": "0.3.0"
    }
  ]

and

-bash-4.2$ cat ./Sample3_Rafael_LM_crop_5_Trans.ome.zarr/s0/.zarray 
{
  "shape": [
    1,
    1402,
    8984
  ],
  "chunks": [
    1,
    512,
    512
  ],
  "fill_value": 0,
  "dtype": ">u2",
  "filters": [],
  "dimension_separator": "/",
  "zarr_format": 2,
  "compressor": {
    "id": "gzip",
    "level": -1
  },
  "order": "C"
}

Note that I changed now according to mobie/mobie-viewer-fiji#572:

"fill_value": "0",. to "fill_value": 0,

Could that be the issue?

[tischi]

I think that's wrong?

public class ZArrayAttributes {

protected static final String zarrFormatKey = "zarr_format";
    protected static final String shapeKey = "shape";
    protected static final String chunksKey = "chunks";
    protected static final String dTypeKey = "dtype";
    protected static final String compressorKey = "compressor";
    protected static final String fillValueKey = "fill_value";
    protected static final String orderKey = "order";
    protected static final String filtersKey = "filters";

    private final int zarr_format;
    private final long[] shape;
    private final int[] chunks;
    private final DType dtype;
    private final ZarrCompressor compressor;
    private final String fill_value;   // <----- MUST be something numeric... double?
    private final char order;
    private final List<Filter> filters = new ArrayList<>();

[constantinpape]

@tischi @KateMoreva can we please add tests to mobie-io that check that the old ome.zarr versions can still be read?

[KateMoreva]

I think the problem was with reading the multiscales attribute.

[KateMoreva]

@tischi, could you please try again? Note that you need new files from 1.2.1-SNAPSHOT.

[tischi]

@KateMoreva I did a git pull from develop, but it still is private final String fill_value;
I think this will not work, because it should be a number, right @constantinpape?

[constantinpape]

@KateMoreva I did a git pull from develop, but it still is private final String fill_value;
I think this will not work, because it should be a number, right @constantinpape?

Yeah, fill value should be a number. (That's the short version, ideally it should be a union type.) This seems to be an issue that we inherit from what n5-zarr is doing. I am not sure if it's a good idea to fix it here without looking into this more closely.
@tischi for now I think you shouldn't replace the string in the fill value line; it's most likely unrelated to the issue you are seeing with the boundary chunks and it just came up because we tried to read the data in python. (This should be fixed of course, but I want to know first why this was a string in the first place, it might be some poor attempt to deal with union types and thus necessary. Sorry, this is a bit too complicated to explain here right now, I would need a zoom to explain the underlying issue.)

[KateMoreva]

@tischi, could you please write the full path to the dataset here? I want to check changes that may fix this problem.

[tischi]

/g/cba/exchange/kimberly/SXAA03648.ome.zarr

[tischi]

It works now @KateMoreva!

[KateMoreva]

@KateMoreva I did a git pull from develop, but it still is private final String fill_value;
I think this will not work, because it should be a number, right @constantinpape?

Yeah, fill value should be a number. (That's the short version, ideally it should be a union type.) This seems to be an issue that we inherit from what n5-zarr is doing. I am not sure if it's a good idea to fix it here without looking into this more closely. @tischi for now I think you shouldn't replace the string in the fill value line; it's most likely unrelated to the issue you are seeing with the boundary chunks and it just came up because we tried to read the data in python. (This should be fixed of course, but I want to know first why this was a string in the first place, it might be some poor attempt to deal with union types and thus necessary. Sorry, this is a bit too complicated to explain here right now, I would need a zoom to explain the underlying issue.)

I opened the PR for a future fix of fill_value #53

[tischi]

(the bug with the boundary chucks is still there though, see mobie/mobie-viewer-fiji#572).

[constantinpape]

(the bug with the boundary chucks is still there though, see mobie/mobie-viewer-fiji#572).

Yes, as I said already: this bug is almost certainly unrelated to the fill_value. I believe that this is likely a problem when reading the boundary chunks; see my latest comments in mobie/mobie-viewer-fiji#572. (And let's keep the discussion about the boundary chunk issue there).

[constantinpape]

xref #52 (comment)

[constantinpape]

We are making progress!

Reading ome.zarr v0.4 works now, except for multiple images per zarr, see #63.

@K-Meech how is the status with the writer, is this working for ome.zarr v0.4 ("final" version) already?

[K-Meech]

@constantinpape - the writers don't support v0.4 yet. I was waiting for the v0.4 readers to be finished first, so it would be easier to test the results from the writers. I'll get on this now! Hopefully I'll have time this week.

[K-Meech]

@constantinpape - I'm just having a quick look through the v0.4 example data. I'm a bit unsure of the metadata for the volumes with time. For these, you have one coordinateTransformations underneath the axes, then one for each dataset.

E.g.

"coordinateTransformations": [
                {
                    "scale": [
                        10,
                        1,
                        1,
                        1,
                        1
                    ],
                    "type": "scale"
                }
            ],

and then for the first dataset:

"coordinateTransformations": [
                        {
                            "scale": [
                                1,
                                1,
                                1.0,
                                0.65,
                                0.65
                            ],
                            "type": "scale"
                        }
                    ],
                    "path": "s0"

If I left out the first one, would it be equivalent to put 10 for the time scale in every dataset? Or is it doing something different?

[constantinpape]

If I left out the first one, would it be equivalent to put 10 for the time scale in every dataset? Or is it doing something different?

The "general" coordinateTransformation applies to each scale the same way. So, in this case, instead of having it one could just put the 10 for the time axis to each of the datasets. Does that answer your question?

[K-Meech]

Yes -thanks @constantinpape!

[constantinpape]

Reading v0.4 is fully implemented now! Thanks @KateMoreva!

[constantinpape]

v0.4 is fully supported now (and everything related to this is released!) :)