Skip to content
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

Coordinate systems and new coordinate transformations proposal #138

Draft
wants to merge 122 commits into
base: main
Choose a base branch
from

Conversation

bogovicj
Copy link
Contributor

@bogovicj bogovicj commented Sep 15, 2022

This PR has four main contributions:

  • The existing axis specification into coordinateSystems - named collections of axes.

  • coordinateTransformations now have "input" and "output" coordinate systems.

  • Adds many new useful type of coordinate transformations.

    • informative examples of each type are now given in the spec.
  • Describes the array/pixel coordinate system (origin at pixel center)

    • as agreed in issue 89 (see below)
  • Adds a longName field for axes

    • Also nice for NetCDF interop

See also:

bogovicj and others added 26 commits February 1, 2022 13:50
* define pixel center coordinate system
* add input/output_axes
* add transform inverse flag
* add affine transform type
* flesh out array space
* define array indexing
* add more transformation types
* start transformation details section and examples
* update example
* use "input" and "output" rather than '*Space' and '*Axes'
* reorder details
* clean up table
* add rotation
* details for sequence
* describe inverses
* wrap examples
* rephrase matrix storage
* change to "coordinates", removing "Field"
* change to "displacements", removing "Field"
* add details for transformation types
* (identity, inverseOf, bijection)
* describe inputAxes and outputAxes
* add new examples
* add mapIndex, mapAXis
* add examples
* affine stored as flat array only
* sequence does not have by-dimension behavior
* flesh out some examples
@github-actions
Copy link
Contributor

github-actions bot commented Sep 15, 2022

Automated Review URLs

@imagesc-bot
Copy link

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

https://forum.image.sc/t/ome-ngff-community-call-transforms-and-tables/71792/1

@d-v-b
Copy link
Contributor

d-v-b commented Feb 16, 2024

is there a strong reason to limit it to 3?

Good point. I wondered the same thing. Personally, I'm happy to remove the restriction, but kept it largely to minimize the number of changes.

I can see a benefit to keeping the restriction in order to catch typos / mistakes. Then folks would be forced to use a different custom type (now allowed), say space12D or whatever.

From the perspective of an implementer of the spec, extra restrictions like "only 3 space axes" require extra lines of code for validation which I would rather not write and test, so without a clear argument for why nobody should ever be allowed to have 4 space axes, I'd suggest removing this restriction (and all the other axis restrictions).

We should probably operate under the assumption that users know their data better than us, and so the spec should impose as few restrictions as possible. If someone needs to describe their data with 4 spatial axes and 2 time axes, then the format should not stop them. Opinionated visualization tools might complain about such data, but that's between the user and the viz tool.

@d-v-b
Copy link
Contributor

d-v-b commented Feb 16, 2024

unless I'm missing something the "C-vs-F-order-for-affine-transformations" conversation seems to have tailed off without a clear resolution. I suspect this dispute is fundamentally intractable -- whatever indexing order we use, someone will be unhappy. This motivates a solution for representing transformations that can be understood without array indexing conventions.

affine transforms without array indexing

Here's one idea, probably someone can improve it:

{
    "transforms" : [
        {
            "type": "matrix",
            "params": {
                "z": {"z": 1, "y": 0.5, "x": 0}, 
                "y": {"z": 0.5, "y": 0.2, "x": 0},
                "x": {"z" : 0, "y": 1, "x": 1} 
            }
        },
        {
        "type": "translation",
        "params": {"z": 1, "y": 1, "x": 1}
}
]
}

This represents a homogeneous affine transformation as a list with two transformations, the first element is a "matrix" transformation, that expresses a linear mapping from named input dimensions to named output dimensions. The second element is a translation transformation, that applies a translation.

Both transformations have a params field that is an object, with keys that are dimension names. So the following two transformations, which use a different key order for the output dimensions, are equivalent:

{
  "type": "translation",
  "params": { "z": 1, "y": 1, "x": 1}
}
{
  "type": "translation",
  "params": {"x": 1, "y": 1, "z": 1}
}

The params field of the "matrix" transformation is composed of key: value pairs like this:

"z": {"z": 1, "y": 0.5, "x": 0}

The key z is the name of the output dimension; the object stored under the key z defines the input, which is itself an object with keys that are the names of input dimensions, and values that are numerical coefficients. We can think of the above JSON as defining a function f(z, y, x) {1 * z + 0.5 * y + 0 * x} and associating the output of that function to a dimension name z.

Personally I find this more readable than the array-of-arrays equivalent, and crucially it completely avoids any array indexing conventions. It also avoids a potentially confusing feature of homogeneous transforms, which is the introduction of an extra dimension in the matrix.

Naming the input and output dimensions has other benefits: you can alter the set of keys in params to drop or add dimensions, without needing to introduce new transformation types:

e.g., adding output dimensions:

{
    "type": "matrix",
    "params": {
        "z": {"z": 1, "y": 0.5, "x": 0}, 
        "y": {"z": 0.5, "y": 0.2, "x": 0},
        "x": {"z" : 0, "y": 1, "x": 1}
        "phi": {"z" : 1, "y": 1, "x": 1} 
    }
}

or dropping them:

      {
          "type": "matrix",
          "params": {
              "z": {"z": 1, "y": 0.5, "x": 0}, 
          }
      }

It doesn't look like a matrix, but that's OK -- a matrix is just one representation of a linear transformation, and the particulars of the matrix representation (which array indexing order? array of arrays or flat array?) is clearly a divisive topic. Honestly, even if everyone uniformly dislikes the JSON I'm proposing above, I think that's a win, because at least everyone is on the same page (provided that the JSON objects I'm proposing can do the same work as a bare array of numbers).

making array indexing conventions more explicit

Even if we resolve the C vs F order dispute for affine transforms, the spec should be more explicit about the array indexing convention used for other dimensional attributes, like the list of Axis objects. One solution is to encode reference dimensional data, i.e. dimension names, in a 1D JSON array that, when reshaped into an ND array and indexed by either F or C native tooling, the correct dimension name comes out. For example:

import numpy as np
# place dimension names, or None, at positions in a 1D space
index_reference = [None, 'x', 'y', None]

# when reshaped into a cube, the dimension names end up at the positions 
# that depend on the array indexing convention 
arr_c = np.array(index_reference).reshape(2,2, order="C")
print(arr_c)
"""
[[None 'x']
 ['y' None]]
"""

arr_f = np.array(index_reference).reshape(2,2, order="F")
print(arr_f)
"""
[[None 'y']
 ['x' None]]
"""

# If I use C-ordered indexing, what is the name for the first axis of the data?
arr_c[-1, 0]
"""
'y'
"""

# If i use F ordered indexing, what is the name for the first axis of the data?
arr_f[-1, 0]
"""
'x'
"""

Thus, a single piece of JSON-serializable data can unambiguously encode the correct mapping from array indices to dimension names. I think the combination of a) adding this data somewhere in the root of the metadata hierarchy, and b) indexing coordinate transformations with dimension names instead of numbers could go a long way towards smoothing out the C / F divide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.