Deltares / xugrid

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

`merge_partitions` fails for datasets with multiple topologies #134

Closed veenstrajelmer closed 7 months ago

veenstrajelmer commented 1 year ago

Merging partitions with multiple topologies (1D2D model) raises: "ValueError: Expected 4 UGRID topologies for 4 partitions, received: [...]"

MWE:

import glob
import xugrid as xu

file_nc = r'p:\1230882-emodnet_hrsm\GTSMv5.0\SO_NHrivGTSM\computations\BD014_fix_mapformat4\output\gtsm_model_0*_map.nc'
file_nc_list = glob.glob(file_nc)
partitions = []
for iF, file_nc_one in enumerate(file_nc_list):
    print(iF+1,end=' ')
    uds_part = xu.open_dataset(file_nc_one)
    partitions.append(uds_part)
uds = xu.merge_partitions(partitions)
JoerivanEngelen commented 7 months ago

Debugged this, it appears the issue stems from the fact that the 1D grid only occurs in one partition, whereas the 2D grid occurs in all four partitions.

What would be appropriate behavior here @Huite : Be strict, as is the current behavior, or allow Datasets with a mix of 2D grids as well as 1D grid in some partitions?

veenstrajelmer commented 7 months ago

For visual reference, this issue contains an image of the grid: https://github.com/Deltares/dfm_tools/issues/497

Huite commented 7 months ago

Good question. It seems reasonable to allow this.

The check is easy to adapt by also checking for length 1:

    n = n_partition
    if not all(len(v) == n or len(v) == 1 for v in grouped.values()):
        raise ValueError(
            f"Expected {n} UGRID topologies for {n} partitions, received: " f"{grouped}"
        )

Maybe everything already just works if you only remove the check?

Probably not, because the grid topologies are zipped with the data_objects. So if the single topology happens to be in the first partition, it'll work, but not if it's in the third.