No separate field for array coordinate system

[jo-mueller]

Fixes ome/ngff#438
Re #18
Re Comment #4

Built on top of #67

Key features:

• Updates schema to allow coordinate systems with axis of only type "array" (previously had to be either 2 or 3 axes of type "space")
• Clarified usage of arrayCoordinateSystems towards usage in explicit fashion: multiscales transformations would need to be specified in pixel units so that the "default" coordinate system is in pixel units and other transforms can then use it as input/output if they're defined in pixel coordinates.
• Added an example to that end

[github-actions]

Automated Review URLs

• Readthedocs

[jo-mueller]

@bogovicj does this sound alright to you?

[bogovicj]

I have one substantial change and a few small edits to consider.

[jo-mueller]

@bogovicj @clbarnes @lubianat I think this is good to go. I updated the description above for key features. If this and #102 are merged we're green for release 👍

[lubianat]

I am not confident on my background to properly review this one, so I am skipping it for the moment, if it is okay.

[jo-mueller]

@btbest @dstansby @will-moore would you mind giving this one a look?

I hope this one makes arraycoordinatesystems a whole lot clearer on how they are supposed to be used. The intended use case was slightly different (in the past), but I hope that the current change of text clarifies things and allows clean usage.

[bogovicj]

Looks good to me, thanks @jo-mueller !

[will-moore]

I had thought that the spec currently states that dimension_names should be set on the zarr array metadata, but now I can't see any mention of this in the v0.6 spec.
In v0.5 at https://ngff.openmicroscopy.org/specifications/0.5/index.html#axes-metadata I see:

The “dimension_names” attribute MUST be included in the zarr.json of the Zarr array of a multiscale level and MUST match the names in the “axes” metadata.

But this has been removed from v0.6? Was there a reason for this or is it simply an omission?

And even in this PR, which is the only mention of dimension_names in the spec, it is only to say they MUST be unique and that the axes names SHOULD correspond to the dimension_names (not MUST).

In v0.5, I would think of the zarr arrays as an "array coordinate system", but in that case the dimension names wouldn't be "dim_0", "dim_1", "dim_2" but would be "t", "c", "z", "y", "x".
Therefore, in v0.6, I would imagine it's possible / desirable to have an array coordinate system, with dimension_names of "t", "c", "z", "y", "x", even if this is before the array is scaled to physical coordinates.
These dimensions may not have physical sizes (units are pixels), but it's still more useful to know the names of the dimensions than to have "dim_0", "dim_1", "dim_2", so I'm not sure why default "dim_n" names should be encouraged?

[jo-mueller]

@will-moore

In v0.5 at https://ngff.openmicroscopy.org/specifications/0.5/index.html#axes-metadata I see:
The “dimension_names” attribute MUST be included in the zarr.json of the Zarr array of a multiscale level and MUST match the names in the “axes” metadata.

Well, that one makes sense to remove, since there can be more than one coordinate system (and thus more than one sets of axes) in the metadata. In the case where multiple coordinate systems were present, it would be ambiguous from which axes to choose the "correct" values for the dimension_names fields.

The motivation here is to make sure that if an arraycoordinatesystem exists, its axes should be the same as the dimension_names in the corresponding zarr array metadata. I'm not fixed on the recommendation for axes names of arraycoordinatesystems to be dim_0, dim_1, etc...as long as they declare that all axes are of type "array" and possibly match the axes names in the dimension_names if they exist.

Edit: With the changes in this PR, I was hoping primarily to clear up the situation on array coordinate systems in a non-conflicting fashion so one could definitely use pixel-based transforms (requires coordinate systems with axes of all type "array") while not overly relaxing the recommendations on the dimension_names, which were important to @bogovicj.

[dstansby]

I've had a read of the new "Array coordinate systems" section, and I don't understand what it's adding to the spec. It seems to me there are two problems array coordinate systems are trying to solve:

1. I want a coordinate system with units of pixels (/voxels/array coordinates)

In the previous section the use of "type": "array" and "discrete": true values is already explained and covers this case

2. I want to refer to the coordinate system of a image array that's stored as a Zarr array

In this case there's already syntax for referring to an 'implict' array coordinate system, and in multiscales you are currently forced to refer to these systems with path, not a named array coordinate system:

The transformation MUST take as input points in the array coordinate system corresponding to the Zarr array at location path. The value of input MUST equal the value of path, implementations should always treat the value of input as if it were equal to the value of path.

So my two cents is

• in the multiscales metadata the 'implicit' array coordinate system must already be referenced with a path
• that still leaves room for users to define their own 'type': 'array' coordinate systems elsewhere
• therefore the whole "Array coordinate systems" could be deleted and would make the spec more readable without sacrificing any functionality

That seems too easy, perhaps I'm missing something...? I have found the discusssions a little tricky to understand without a concrete example of a problem the array coordinate system metadata is intended to solve.

[btbest]

(I know I know, bullet points, but I promise I wrote this myself by hand 😅 )

Good:

• Where array coordinate systems are supposed to go is now clear (and thank you very much for not putting it in the array zarr.json, which is what it sounded like to me)
• Multiscale metadata is now allowed to contain "array" axes, actually enabling the intended use-case (previously would only have been solvable via scene; awkward)
• Example illustrates what one might want to do with this

Could be better:

• Use case imo not well-defined enough to warrant such an extensive doc in the spec (feels immature compared to the rest of the RFC)
• Example directly contradicts what we've been teaching people to do with their pixel size metadata

Suggestion, even if maybe a bit radical:

• Axe the entire descriptive "array coordinate systems" section and example, leave only the modification to the multiscale spec change.
• Add a statement there to make it clear that this is non-standard but allowed
• And/or add that if the intrinsic system has "array"-type axes, the multiscale coordinate systems SHOULD also contain a system with "space"-type axes and there SHOULD be a multiscale-level transform of scale-type with input=intrinsic and output=physical.

[btbest]

OME-Zarr doesn't currently encode a concept of points or ROIs, which makes any such definitions custom structures. How a user's custom structures map to OME-Zarr coordinate systems is for them to encode alongside their custom structures imo.

[btbest]

can be explicitly defined in the list of coordinate systems

Whose? Scene, Multiscale, both? (probably super minor, could just use "any" instead of "the")

[btbest]

Suggested change

	as the primary target of the multiscales coordinate transformations.
	as the "intrinsic" coordinate system of a multiscale. The more conventional "physical" coordinate system is provided as an optional alternative, with a "scale"-type transformation connecting the two.

[btbest]

Suggested change

A single coordinate transformation (`scale` at the bottom) is then used to project the image into the `physical` coordinate system:

[btbest]

In the example, the array coordinate system is now the "intrinsic" system. Given the emphasis on the concept of an "intrinsic" system in the multiscale spec, I have understood that the "intrinsic" system should be treated as the "default" system. Imo the default should be a physical coordinate system; it's the conventional approach and more intuitive.

The multiscale section of the spec does currently describe that the intrinsic system should represent physical space, not just an arbitrary common system (even if it doesn't use SHOULD/MUST).

It's a matter of deciding which, but the spec should be internally consistent:

• Option 1: Intrinsic means nothing. Remove the "intrinsic" terminology from multiscale metadata spec and instead just require "all dataset transforms map to one common coordinate system". Maybe add "readers SHOULD default to a coordinate system that uses physical axes (type=space)"? The example here can stay as it is.
• Option 2: Intrinsic is meaningful and readers should default to using it. Multiscale spec can stay as is. Modify the example so that the physical coordinate system is the intrinsic one. I.e. dataset transformations as conventional, and then the multiscale-level transformation to the array coordinate system is the inverse of the intrinsic system's native pixel size.

I'd go for option 2.

[will-moore]

I was under the impression that "intrinsic" coordinate system was a concept proposed for v0.6dev2 where we needed some rules about which coordinate system to use when referring to a multiscales image from a parent "scene".
But since v0.6dev3, a transform in a scene must use "input": {"path": "image.zarr", "name": "physical"} to explicitly refer to a coordinateSystem. So we don't need the concept of "intrinsic" coordinate System any more?

I see that "intrinsic" coordinateSystem is still mentioned a few times in https://ngff.openmicroscopy.org/specifications/dev/index.html - when describing various rules for the "graph" of transforms and systems. But I thought that we'd decided the only rule we needed was that there shouldn't be unconnected parts of the graph, as described under https://ngff.openmicroscopy.org/specifications/dev/index.html#additional-details

[jo-mueller]

I think I'm fine with the usage of the term "intrinsic" coordinate system. It's just an alias that refers to the coordinate system that all multiscale transformations use as output, without imposing any particular requirements as to how this coordinate system is used. The only hard rules are:

• All multiscales transformations MUST use output to the same coordinate system (i.e., the "intrinsic" system)
• All additional coordinate transformations inside the multiscales metadata MUST have this "intrinsic" coordinate system as input, which I think is also reasonable.

Furthermore, the current spec states:

This coordinate system (the “intrinsic” coordinate system) will generally be a representation of the image in its native physical coordinate system. It should be used for viewing and processing unless a use case dictates otherwise.

Which I think pretty much nails it. It is perfectly ok if a use case dictates the existence of a coordinate system in pixel units for transformations to tap into. You can still attach a transformation that transforms from this "intrinsic" system into a physical coordinate system.

[imagesc-bot]

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/ngff-weekly-dev-update-thread/110810/76