Prevent "band math" like attempts with `filter_bands`
Spin-off from https://github.com/Open-EO/openeo-python-client/issues/76#issuecomment-923896434 :
User might mistakenly use filter_bands() instead of bands() to do band math: e.g.
green = cube.filter_bands("green")
nir = cube.filter_bands("nir")
ndwi = (green - nir) / (green + nir)
This will currently be translated to 3 merge_cubes operations with an overlap resolver (one for each of the three math operators). Not only is this very inefficient, it will also give unexpected results.
We should raise error or warning if we detect this (doing math operation on filter_bands result)
it will also give unexpected results.
If I'm not mistaken, this is what should happen in pseudo code:
-
green - nir->merge_cubes(cube1="filter_green", cube2= "filter_nir"}, overlap_resolver="subtract")Because band labels are different there is no overlap to be resolved and the result will be 4D datacube with the original green and nir bands -> datacube "diff" -
green + nir-> same reasoning: result will be 4D datacube with the original green and nir bands -> datacube "sum" -
diff / sum->merge_cubes(cube="diff", cube2="sum", overlap_resolver="divide"), now because "diff" and "sum are actually identical, there is overlap to be resolved by "divide" operation results in always 1 (unless input is zero, then it's NaN)
final result will be a 4D datacube (2 bands) with 1 values everywhere :0
For a bit more context:
when the client works with a "math" expression like cube1 + cube2:
- if both data cubes come from a
.band()operation, "band math" mode is used:reduce_dimensionalong "bands" dimension with a reducer that implements the operation (addin this case) - otherwise (e.g. cubes come from
.filter_bands()or something else),merge_cubesis used with an overlap resolver that implements the operation
The first case works fine and is a handy feature.
The second case is more tricky because there is no guarantee that the overlap reducer will be used at all (it depends on the overlap between the cubes), so the results might be not intuitive or hard to explain.
For example if the math expression is cube1 - cube2, the output values of the result will be:
-
cube1where onlycube1is defined -
cube1 - cube2where there is overlap -
cube2where onlycube2is defined
Users probably have different expectations: for example: only have cube1-cube2 where there is overlap and leave the rest NaN, or at least have 0 - cube2 where only cube2 is defined.
I think we should reconsider using merge_cubes for implementing math operators in a non-band-math context.
FYI as far as I can remember, merge_cubes based math was introduced to have easy to read mask operations like
scene_classification = con.load_collection("SCENECLASSIFICATION", ...)
mask = (scene_classification == 4) | (scene_classification == 5)
which works without issues mentioned above because there is 100% overlap
Some options to resolve this whole issue:
- only support
merge_cubesbased math for logical operators (and, or, not), but drop support for standard math operators (+, -, * ...) - try to detect in client if overlap is 100%, and throw an error, so that there are no surprises
- drop
merge_cubesbased math all together and promote explicit usage ofmerge_cubeswithProcessBuilderfor the overlap resolver
what do you think @jdries ?
Having an "overlap-only" version of merge_cubes might also be a solution: see https://github.com/Open-EO/openeo-processes/issues/280
I seem to be mostly in favor of trying to detect this case on the client side. A substraction between two cubes with the same number of bands, and different band names, should not result in a cube with 2 times the number of bands. I think that's at least how the user would expect it to work.
So throwing an error is fine for me. There's possibly also an option to introduce even more logic in the client to resolve the case, but that might be too much magic?
Another user support issue that, after hours of digging, again turned out out to be about unexpected results from merge_cubes with an unused overlap resolver: https://forum.dataspace.copernicus.eu/t/inconsistent-rbr-pixel-values-using-openeo-sentinel-2-data/3804