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

Preserve nan in comparisons #527

Open
soxofaan opened this issue Oct 2, 2023 · 8 comments
Open

Preserve nan in comparisons #527

soxofaan opened this issue Oct 2, 2023 · 8 comments
Assignees
Labels

Comments

@soxofaan
Copy link
Member

soxofaan commented Oct 2, 2023

The openEO processes explicitly mention nan-support for gt, lt, gte, ...:

... If any operand is null, the return value is null ...

So the output of such a process could be true, false and no-data.
I think we work purely binary and cast (?) no-data to false

(brought up here by @clausmichele : https://discuss.eodc.eu/t/boolean-mask-and-mask-polygon-issue/626)

@soxofaan soxofaan added the bug label Oct 2, 2023
@soxofaan
Copy link
Member Author

soxofaan commented Oct 2, 2023

created minimal reproduction use case here:
https://gist.github.com/soxofaan/aaf001c8ee6aca4198d1475b9ca8741e

Note how the nan from the mask-polygon results in different outputs:

  • in binary mask: value 0 from casting to false, value 129 in all-nan tiles that were eliminated before comparison
  • float mask: value 0 from casting to false, value nan in all-nan tiles that were eliminated before comparison

@EmileSonneveld
Copy link
Contributor

Numpy gives a boolean result when comparing with nan (JavaScript does the same).

import numpy as np
print(np.__version__)  # 1.24.4
print(np.nan == 3)  # False
print(np.nan != 3)  # True
print(np.nan <  3)  # False
print(np.nan <= 3)  # False
print(np.nan >  3)  # False
print(np.nan >= 3)  # False

Maybe we could pass the nan behaviour as an option? For example: ndsi.lt(0, preserve_nan=True)

@EmileSonneveld
Copy link
Contributor

There might be more wrong than just that. Reading up a bit more...

@clausmichele
Copy link
Member

@EmileSonneveld please have a look at this, since it's highly related: https://github.com/Open-EO/openeo-processes-dask/blob/main/docs/decisions/handle-nodata-in-rastercubes.md

@EmileSonneveld
Copy link
Contributor

Exporting a signed byte layer results a tiff or nc makes a file that gets interpreted as a unsigned byte image by Q-gis and rasterio.

An example for this bug is visible in an auto test here: https://jenkins.vgt.vito.be/job/openEO/job/openeo-geotrellis-extensions/job/develop/781/execution/node/3/ws/openeo-geotrellis/outFiltered.tif (Tiles at the left are 128)

It looks like when exporting a 'bit' layer to netcdf, it ends up as a byte layer, and probably triggering this bug. Explaining the 128 values instead of nodata in the first example
image

@EmileSonneveld
Copy link
Contributor

As a workaround, it is already possible to preserve nan like this:

snowmap = ndsi.apply(lambda x: if_(is_nan(x), x, x > 0.4))

EmileSonneveld added a commit to Open-EO/openeo-geotrellis-extensions that referenced this issue Nov 20, 2023
EmileSonneveld added a commit to Open-EO/openeo-geotrellis-extensions that referenced this issue Nov 21, 2023
EmileSonneveld added a commit to Open-EO/openeo-geotrellis-extensions that referenced this issue Nov 21, 2023
EmileSonneveld added a commit to Open-EO/openeo-geotrellis-extensions that referenced this issue Nov 21, 2023
@EmileSonneveld
Copy link
Contributor

The following image demonstrates 2 issues when exporting int8 variations to tiff. At the right side, it shows how the fix of cast_byte_to_short looks like.

int8_bug_proof

@EmileSonneveld
Copy link
Contributor

GDAL >= 3.7 supports signed bytes: https://github.com/OSGeo/gdal/blob/7d5725c91eafc9fb0fc2c3d4136a466004648d14/gcore/gdal.h#L67C55-L67C55 (From May 2023)
But Q-Gis 3.22.4 uses GDAL 3.4.1
GDAL version on Jenkins server: 2.3.2
GDAL I have running locally: 3.4.1

Until GDAL 3.7< is used everywhere, int8 could cause overflow issues on many locations.

EmileSonneveld added a commit to Open-EO/openeo-geotrellis-extensions that referenced this issue Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants