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

solve edge_node_connectivity in xugrid 0.6.0 #113

Closed
veenstrajelmer opened this issue Jul 6, 2023 · 0 comments · Fixed by #114
Closed

solve edge_node_connectivity in xugrid 0.6.0 #113

veenstrajelmer opened this issue Jul 6, 2023 · 0 comments · Fixed by #114

Comments

@veenstrajelmer
Copy link
Collaborator

veenstrajelmer commented Jul 6, 2023

Reading a ugrid file with triangles and squares results in several errors in xugrid==0.6.0:
"ValueError: Invalid edge_node_connectivity"
"ValueError: triangles are indices into the points and must be in the range 0 <= i < 3101 but found value -2"

This probably has to do with the fact that the dflowfm connectivities are 1-based instead of the default 0-based and a testcase must be missing for this situation:

Attributes:
    cf_role:      edge_node_connectivity
    start_index:  1
    _FillValue:   0

MWE:

import xugrid as xu
file_nc = 'Grevelingen-FM_0002_map.nc' #dflowfm map output with 1-based connectivity and mix of triangles/squares
uds = xu.open_dataset(file_nc)

uds.isel({uds.grid.face_dimension:[1]}) # raises "ValueError: Invalid edge_node_connectivity"
uds.mesh2d_node_z.ugrid.plot() # raises "ValueError: triangles are indices into the points and must be in the range 0 <= i < 3101 but found value -2"

Additional info
All connectivity vars have start_index 1 in this file, when printing

uds_grid_ds = uds.grid.to_dataset()
ds_startidx1 = uds_grid_ds.filter_by_attrs(start_index=1)
for varn_conn in ds_startidx1.data_vars:
    mesh2d_attrs = uds_grid_ds.mesh2d.attrs
    connect_name = list(filter(lambda x: mesh2d_attrs[x] == varn_conn, mesh2d_attrs))[0]
    print(f'connectivity in {varn_conn} ({connect_name})')
    print('start_index', ds_startidx1[varn_conn].attrs["start_index"])
    print('min()', ds_startidx1[varn_conn].min().to_numpy())

in xugrid 0.5.0 this gives:

connectivity in mesh2d_face_nodes (face_node_connectivity)
start_index 1
min() 1.0
connectivity in mesh2d_edge_nodes (edge_node_connectivity)
start_index 1
min() 1.0
connectivity in mesh2d_edge_faces (edge_face_connectivity)
start_index 1
min() 0.0

in xugrid 0.6.0 this gives:

connectivity in mesh2d_face_nodes (face_node_connectivity)
start_index 1
min() -1.0
connectivity in mesh2d_edge_nodes (edge_node_connectivity)
start_index 1
min() 1.0
connectivity in mesh2d_edge_faces (edge_face_connectivity)
start_index 1
min() 0.0

The face_node_connectivity is messed up here, with a start_index of 1 the minimum value should be 1 or 0 (missing value). Could it be that there was a minus-to-plus-1 (or the other way round) introduced somewhere in the code? I cannot find a lot about face_node_connectivity in the diff though. Also interesting, when opening a partition with only triangles, the minimum of face_node_connectivity is 1 (so correct).

This issue was introduced by fixing #101

testcase suggestion
Suggestion to add a testcase with e.g. the sandiego dataset, regridding it to a mesh with triangles+squares. And with these edits to the grid before putting it in a new UgridDataset and plotting it:

ds_idx = xu_grid_ds.filter_by_attrs(start_index=0)
for varn_conn in ds_idx.data_vars:
    xu_grid_ds[varn_conn] += 1
    xu_grid_ds[varn_conn].attrs["_FillValue"] += 1
    xu_grid_ds[varn_conn].attrs["start_index"] += 1
veenstrajelmer added a commit to Deltares/dfm_tools that referenced this issue Jul 6, 2023
veenstrajelmer added a commit to Deltares/dfm_tools that referenced this issue Jul 6, 2023
* updated requirements: meshkernel>=2.1.0
* excluded xugrid 0.6.0 because of a bug (Deltares/xugrid#113)
* improved formatting of test script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant