Skip to content

Conversation

@browniea
Copy link
Contributor

@browniea browniea commented Nov 18, 2025

Updates to io/d3d.py code

  • Added new coordinate names for the latest version of Delft3D
  • new calculate_grid_convergence_index function to calculate the grid convergence index between two grid's results basted on the equation GCI
  • Allows xarray or netCDF4 input

@browniea browniea requested a review from akeeste November 19, 2025 19:19
@browniea
Copy link
Contributor Author

Hey @akeeste I added the updates for the updated variable names in the Delft3D 2025 version. I was thinking about updating the code to also accept xarry in addition to netCDF.

@browniea browniea marked this pull request as ready for review January 28, 2026 19:11
@browniea
Copy link
Contributor Author

Hey @akeeste pending all the checks pass, this is ready for review.

@akeeste akeeste self-assigned this Jan 28, 2026
@akeeste
Copy link
Contributor

akeeste commented Jan 28, 2026

Thanks @browniea I'll look for some time to review next week

Copy link
Contributor

@akeeste akeeste left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @browniea! I have a few minor requests to fine tune this PR and then we can merge

gci: float
Grid Convergence Index (GCI).
"""
# Calculate the approximate relative error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add basic type checks (all inputs are numerical, fine_grid and coarse_grid are the same shape, etc)

if not isinstance(data, netCDF4.Dataset):
raise TypeError("data must be a NetCDF4 object")
if not isinstance(data, (netCDF4.Dataset, xr.Dataset)):
raise TypeError("data must be a NetCDF4 object or xarray Dataset")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test for xarray input

)
waterlevel = np.ma.getdata(data.variables["mesh2d_s1"][time_index, :], False)
coords = str(data.variables["waterdepth"].coordinates).split()
coords = str(data.variables["mesh2d_waterdepth"].coordinates).split()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple questions on this change:

  • When did this variable name become relevant
  • Is the old waterdepth variable name still relevant?

):
"""
Calculate the Grid Convergence Index (GCI) between two grid sizes. https://www.grc.nasa.gov/WWW/wind/valid/tutorial/spatconv.html

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add the full citation to the above documentation in case the link ever breaks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants