-
Notifications
You must be signed in to change notification settings - Fork 49
D3D updates #428
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
base: develop
Are you sure you want to change the base?
D3D updates #428
Conversation
|
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. |
|
Hey @akeeste pending all the checks pass, this is ready for review. |
|
Thanks @browniea I'll look for some time to review next week |
akeeste
left a comment
There was a problem hiding this 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 |
There was a problem hiding this comment.
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") |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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
waterdepthvariable name still relevant?
| ): | ||
| """ | ||
| Calculate the Grid Convergence Index (GCI) between two grid sizes. https://www.grc.nasa.gov/WWW/wind/valid/tutorial/spatconv.html | ||
|
|
There was a problem hiding this comment.
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
Updates to
io/d3d.pycodecalculate_grid_convergence_indexfunction to calculate the grid convergence index between two grid's results basted on the equation GCI