search

UXARRAY / uxarray

Xarray extension for unstructured climate and global weather data analysis and visualization.
https://uxarray.readthedocs.io/
Apache License 2.0
148 stars 31 forks source link

`_standardize_connectivity` alters fill values after replacing fill values #853

Closed kjnam closed 1 month ago

kjnam commented 1 month ago

Version

v2024.06.0

How did you install UXarray?

Conda

What happened?

When the start_index is not zero, _standardize_connectivity subtracts the start_index from the connectivity in order to convert the connectivity into zero-based indices. The subtraction, however, occurs after replacing fill values by _replace_fill_values, which alters the new fill values, at https://github.com/UXARRAY/uxarray/blob/e36030bc5e4b6fcc37cc2b3409d353e1483d107e/uxarray/io/_ugrid.py#L150. This effectively corrupts the connectivity fill values when an original data set uses non-zero start index.

What did you expect to happen?

The function _standardize_connectivity should keep the new fill values while subtracting start_index.

Can you provide a MCVE to repoduce the bug?

No response

rajeeja commented 1 month ago

@kjnam thanks for pointing this out, i saw something similar recently. It'd be great if you share how did you stumble upon this?

thanks!

kjnam commented 1 month ago

@rajeeja Our dataset uses one-based indices, following the Fortran index convention. When I try to open one of our dataset using uxarray, the standardize function tries to coerce the connectivity indices to zero base with this bug. The incorrect fill values cause failures in topology aggregations that rely on the connectivity indices.

rajeeja commented 1 month ago

@rajeeja Our dataset uses one-based indices, following the Fortran index convention. When I try to open one of our dataset using uxarray, the standardize function tries to coerce the connectivity indices to zero base with this bug. The incorrect fill values cause failures in topology aggregations that rely on the connectivity indices.

gotcha, thanks! can you share your dataset?

kjnam commented 1 month ago

@rajeeja Will do later. Thanks.