Deltares / xugrid

Xarray and unstructured grids
https://deltares.github.io/xugrid/
MIT License
53 stars 8 forks source link

`xu.core.wrap.UgridDataset` corrupts face_node_connectivity in original `xr.Dataset` #208

Closed veenstrajelmer closed 5 months ago

veenstrajelmer commented 5 months ago

face_node_connectivity in original dataset is corrupted by xu.core.wrap.UgridDataset. The minimal value is 1 before, and -999 (the fillvalue) after wrapping it. This is normally not a problem, but in some of the workflows we wrap them preliminary to identify hanging edges, before we do some other actions on the dataset (like removing these edges) and than wrap it again. If we deepcopy the ds before we wrap it, no issue occurs.

To reproduce Minimal reproducible example:

import xarray as xr
import xugrid as xu

file_nc = r'p:\11209231-001-dwsm-slib2023\02_Modelling\04_Mud\Mud_v2_20162017_rst2\DFM_OUTPUT_DWSM-FM_200m\DWSM-FM_200m_0000_map.nc'
ds = xr.open_dataset(file_nc)
fnc = ds[ds.mesh2d.attrs["face_node_connectivity"]]
print(fnc.min(), fnc.encoding["_FillValue"]) # array(1.) -999
ds_copy = ds.copy(deep=True)
uds = xu.core.wrap.UgridDataset(ds) # if we use ds_copy instead of ds, there is no problem
fnc = ds[ds.mesh2d.attrs["face_node_connectivity"]]
print(fnc.min(), fnc.encoding["_FillValue"]) # array(-999.) -999

uds = xu.core.wrap.UgridDataset(ds)
print(uds.grid)

Expected behaviour No alteration of input ds by xu.core.wrap.UgridDataset

Additional info This was initially documented in https://github.com/Deltares/dfm_tools/issues/680, but that issue did not yet contain a minimal xugrid/xarray example.

Huite commented 5 months ago

Seems easy to fix, fortunately. When calling from_dataset, a _prepare_connectivity method is called in Ugrid1d, Ugrid2d. https://github.com/Deltares/xugrid/blob/a140d3144c483e3e4d8292f12fd03dea435b3814/xugrid/ugrid/ugridbase.py#L385

The problem is this line: https://github.com/Deltares/xugrid/blob/a140d3144c483e3e4d8292f12fd03dea435b3814/xugrid/ugrid/ugridbase.py#L392

If we use data.to_numpy().copy() here, we should be good?