kernel orientation in apply_kernel

[soxofaan]

openeo-processes/apply_kernel.json

Line 20 in 9ee7b1e

"description": "A two-dimensional weighted kernel. Each dimension of the kernel must have an uneven number of elements, otherwise the process throws a `KernelDimensionsUneven` error.",

I think the spec should specify explicitly how the levels of the nested array correspond with the x and y dimension.

I think in most languages (python/numpy, matlab, ...), the inner level corresponds with the x dimension and outer level corresponds with the y dimension, because that is the most intuitive way to render it:

array([[1, 2],
       [3, 4]])

But I don't think we should let this open for interpretation

[m-mohr]

Good point. One could also argue it should be based on the given CRS though.

[soxofaan]

initial quick proposal:

>             "description": "Weighted kernel as a two-dimensional array. The inner level of the nested array corresponds to the `x` dimension and the outer level corresponds to the `y` dimension. Each dimension of the kernel must have an uneven number of elements, otherwise the process throws a `KernelDimensionsUneven` error.",

[soxofaan]

One could also argue it should be based on the given CRS though

Or to drive this even further: why limit this to 2-D spatial? Why not also support use cases with data cubes with 3 spatial dimensions or just 1? And why not also allow convolution along temporal dimension?
This could be addressed by adding a parameter, say dimensions, that is a list of dimension names corresponding with the levels of the nested kernel array (so e.g. ["y", "x"] by default if you use outer-to-inner order)

[m-mohr]

We allowed more dimensions in apply_kernel previously, but we changed that for a reason I can't clearly remember. Maybe just for simplicity.

Related issues/PRs: #69 / #118

[soxofaan]

yeah I thought that would have been considered before. I'm fine with 2-D for now, I guess it can be expanded later in backwards compatible fashion

[soxofaan]

Side track while we're at it: the summary is pretty vague IMHO:

openeo-processes/apply_kernel.json

Line 3 in 9ee7b1e

"summary": "Apply a kernel to compute pixel-wise values",

I would at least include the key word "convolution" there, e.g.

Apply a convolution with a kernel

[m-mohr]

I'm happy to accept PRs for any of the suggested changes.