Skip to content

D3D updates#428

Merged
akeeste merged 35 commits into
MHKiT-Software:developfrom
browniea:D3D_updates
Apr 16, 2026
Merged

D3D updates#428
akeeste merged 35 commits into
MHKiT-Software:developfrom
browniea:D3D_updates

Conversation

@browniea

@browniea browniea commented Nov 18, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

browniea commented Nov 19, 2025

Copy link
Copy Markdown
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

browniea commented Jan 28, 2026

Copy link
Copy Markdown
Contributor Author

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

@akeeste akeeste self-assigned this Jan 28, 2026
@akeeste

akeeste commented Jan 28, 2026

Copy link
Copy Markdown
Contributor

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

akeeste
akeeste requested changes Feb 3, 2026
View reviewed changes

@akeeste akeeste left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread mhkit/river/io/d3d.py
Comment thread mhkit/river/io/d3d.py Outdated
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")

@akeeste akeeste Feb 3, 2026

Copy link
Copy Markdown
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

Comment thread mhkit/river/io/d3d.py Outdated
Comment thread mhkit/river/io/d3d.py
@browniea

browniea commented Feb 6, 2026

Copy link
Copy Markdown
Contributor Author

@akeeste The xarry input is a little more complicate than I initially thought. It might be worth adding an example. I'll plan to have a few functions converted for you to look at our next meeting.

The rest of the changes are done.

@akeeste

akeeste commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

@browniea I checked your recent changes and pulled in dev. This looks good so far.

The last remaining item is to add a test for d3d.py that uses xarray input. I don't think we need a full example--perhaps just duplicating an existing test that uses netcdf and inputting an xarray Dataset instead

@akeeste

akeeste commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

I also pulled develop into this branch. Might have to do so again when #435 is merged

@browniea

browniea commented Feb 26, 2026

Copy link
Copy Markdown
Contributor Author

Making a note here that currently there is some redundancy processing both xarray and netCDF file formats. For now, I'm leaving the redundancy but, in the future, we could convert the input format instead of processing both. D3D outputs netCDF files although xarray has some advantages tabulating the data.

@browniea

browniea commented Mar 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Final few checks

  • Tanana D3D example notebook
  • Mesh2D data format
  • Confirm test_io_d3d covers the mesh2d variable names
  • updated example notebook to include xarray input

@browniea

browniea commented Mar 5, 2026

Copy link
Copy Markdown
Contributor Author

@akeeste, There's no difference between the xarray and netcdf besides how the data is read in, so I don't think it's necessary to do an example notebook. I'll update the current example notebook's descriptions to include the xarray capability. However, the next example I do, #429, can be in xarray.

@browniea

browniea commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

Hey @akeeste This is ready to review. I did get an error from the test_io_d3d.py gave me an error I think coming from the hindcast. The error was it couldn't find the distutils module. I also got some nan results from the mesh_2d data set I tested but it compiled so I think that's an issue with the data I was using. I'll make sure both of these get resolved. Thanks!

akeeste
akeeste approved these changes Mar 12, 2026
View reviewed changes

@akeeste akeeste left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @browniea! This looks good now. As soon as #436 is in, we'll rerun these tests and merge

Comment thread mhkit/tests/river/test_io_d3d.py
Comment on lines +370 to +378
def test_calculate_grid_convergence_index(self):
fine_grid = np.array([1.0, 2.0, 3.0])
coarse_grid = np.array([0.5, 1.5, 2.5])
refinement_ratio = 2.0
gci = river.io.d3d.calculate_grid_convergence_index(
fine_grid, coarse_grid, refinement_ratio
)
assert_array_almost_equal(gci, np.array([0.2083, 0.1042, 0.0694]), decimal=3)

@akeeste akeeste Mar 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this test is failing, but I'm pretty sure it's an issue with the install that #436 should address

@simmsa

simmsa commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

@browniea, I added the latest changes from the develop branch into your PR:

I see one failing test https://github.com/MHKiT-Software/MHKiT-Python/actions/runs/24349977070/job/71168836922#step:10:255:

.github/workflows/mhkit/tests/river/test_io_d3d.py::TestIO::test_calculate_grid_convergence_index - AttributeError: module 'mhkit.river.io.d3d' has no attribute 'calculate_grid_convergence_index'

And some linting errors https://github.com/MHKiT-Software/MHKiT-Python/actions/runs/24349977059/job/71101336823:

mhkit/river/io/d3d.py:1:0: C0302: Too many lines in module (1045/1000) (too-many-lines)
mhkit/river/io/d3d.py:51:11: E0606: Possibly using variable 'seconds_run' before assignment (possibly-used-before-assignment)
mhkit/river/io/d3d.py:372:19: E0606: Possibly using variable 'cords_to_layers' before assignment (possibly-used-before-assignment)
mhkit/river/io/d3d.py:372:35: E0606: Possibly using variable 'layer_dim' before assignment (possibly-used-before-assignment)
mhkit/river/io/d3d.py:399:59: E0606: Possibly using variable 'bottom_depth' before assignment (possibly-used-before-assignment)
mhkit/river/io/d3d.py:400:58: E0601: Using variable 'waterlevel' before assignment (used-before-assignment)
mhkit/river/io/d3d.py:421:37: E0606: Possibly using variable 'layer_percentages' before assignment (possibly-used-before-assignment)
mhkit/river/io/d3d.py:436:29: E0606: Possibly using variable 'time' before assignment (possibly-used-before-assignment)
mhkit/river/io/d3d.py:440:29: E0606: Possibly using variable 'y' before assignment (possibly-used-before-assignment)
mhkit/river/io/d3d.py:826:19: E0606: Possibly using variable 'cords_to_layers' before assignment (possibly-used-before-assignment)
mhkit/river/io/d3d.py:826:35: E0606: Possibly using variable 'layer_dim' before assignment (possibly-used-before-assignment)
mhkit/river/io/d3d.py:841:23: E0606: Possibly using variable 'layer_percentages' before assignment (possibly-used-before-assignment)
mhkit/river/io/d3d.py:786:12: W0612: Unused variable 'bottom_depth' (unused-variable)
mhkit/river/io/d3d.py:787:12: W0612: Unused variable 'waterlevel' (unused-variable)
mhkit/river/io/d3d.py:788:12: W0612: Unused variable 'coords' (unused-variable)

Let us know if you have questions on how to fix the failing test or pylint linting errors.

@browniea

browniea commented Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

Hey @akeeste I'm getting some errors with the 'test_get_buoy_metadata' function. Have you seen this anywhere else?

@akeeste

akeeste commented Apr 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hey @akeeste I'm getting some errors with the 'test_get_buoy_metadata' function. Have you seen this anywhere else?

@browniea I have not. But your changes should not be affecting that test so let's ignore it for now. Aside from that, it looks like everything but one small pylint test is now passing!

@browniea

browniea commented Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

Hey @akeeste I'm getting some errors with the 'test_get_buoy_metadata' function. Have you seen this anywhere else?

@browniea I have not. But your changes should not be affecting that test so let's ignore it for now. Aside from that, it looks like everything but one small pylint test is now passing!

@akeeste I skipped the last pylint error. If that's of with you this is ready to merge.

@browniea browniea mentioned this pull request Apr 16, 2026
Closed
@akeeste

akeeste commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Thanks @browniea ! Merging

@akeeste akeeste merged commit 352a99e into MHKiT-Software:develop Apr 16, 2026
52 of 59 checks passed
@simmsa simmsa mentioned this pull request Jun 29, 2026
Merged
3 tasks
simmsa added a commit that referenced this pull request Jul 8, 2026
@simmsa
Release: Prepare MHKiT-Python v1.1.0 (#449)
31f3c78 
# MHKiT-Python v1.1.0

## Additions

* Acoustics: Add millicdecade and WISPR instrument support
  * Added millidecade spectral conversion
  * Added a WISPR hydrophone reader
  * Added a voltage-based `export_audio` resampling option
  * Refactored band-averaging to avoid losing information at frequency-band boundaries
  * #447
  * Author: @jmcvey3
  * Reviewer: @simmsa
* DOLfYN: Add [Nortek Aquadopp](https://www.nortekgroup.com/oceanography/aquadopp-series) ADCP support
  * Added DOLfYN support for reading Aquadopp instruments
  * Cleaned Nortek parsing code
  * Simplified handling of the non-cabled ADV orientation flag
  * #434
  * Author: @jmcvey3
  * Reviewer: @akeeste
* Examples: Add ADCP waves example: `example/adcp_waves_example.ipynb`
  * Added an example notebook showing how to ingest and analyze wave measurements from a dual-profile Nortek Signature 250 deployment at PacWave.
  * #430
  * Author: @jmcvey3
  * Reviewer: @akeeste

## Improvements

### Acoustics

- Improved flexibility and robustness of the Acoustics module
  - Added a configurable FFT length for sound pressure PSDs
  - Renamed bin/windowing attributes for clarity
  - Fixed an incorrectly signed gain correction
  - #433
  - Author: @jmcvey3
  - Reviewer: @akeeste

### DOLfYN

- Refactored PSD calculations to use `scipy.signal.welch`
  - Replaced DOLfYN's custom Welch-like PSD implementation (built each segment from bin slices, could not overlap FFT segments) with `scipy.signal.welch`
  - Removed the bin-based segment structure and added a `step` argument to control overlap
  - Dropped float32 casts in some functions in favor of float64
  - Removed `fft.py` and renamed `tools/misc.py` to `tools.py`
  - Breaking Changes:
    - NaNs are no longer tolerated in PSD calculations
    - DOLfYN defaults to 0% overlap to preserve existing array shapes
    - Acoustics defaults to 50% overlap
    - PSD output time dimension renamed to `time_psd`
  - #452
  - Author: @jmcvey3
  - Reviewer: @akeeste, @simmsa
- Improved handling of "averaged" profiles
  - Fixed handling of Nortek Signature dual-profile ADCP data by defaulting to "_avg" velocity variables when untagged ones are absent.
  - #430
  - Author: @jmcvey3
  - Reviewer: @akeeste
- Critical PSD bugfix
  - Fixed a bug where individual FFTs received a 50% overlap twice, which corrupted the first and last spectrum of a timeseries.
  - #430
  - Author: @jmcvey3
  - Reviewer: @akeeste

### Examples

- Added histograms to ADCP example
  - #448
  - Author: @browniea
  - Reviewer: @akeeste


### River/IO

- Fixed Qhull interpolation and D3D coordinate-system errors
  - #448
  - Author: @browniea
  - Reviewer: @akeeste
  - Fixes: #442, #444
- Delft3D module updates
  - Added new Delft3D coordinate names
  - Added a new grid-convergence-index calculation function
  - Added support for xarray/netCDF4 input in the D3D module
  - #428
  - Author: @browniea
  - Reviewer: @akeeste

### Wave

- NDBC Directional Wave Units
  - Fixed NDBC directional wave spectrum output to return degrees instead of radians
  - Updated polar plots so 0 deg is at the top and increases clockwise
  - #437
  - Author: @jmcvey3
  - Reviewer: @akeeste
  - Fixes: #427

### Wave/Hindcast

- Added a `hindcast_guard` exception-handling decorator (`hindcast_exceptions.py`) that surfaces a clear error on HSDS request failures, distinguishing the known NLR HSDS outage (#450) from other failures
  - #449
  - Author: @simmsa
  - Reviewer: @akeeste

## Maintenance

- Added Python 3.13 support
  - #445
  - Author: @simmsa
  - Reviewer: @akeeste
  - Fixes: #441
- Added [pandas 3](https://pandas.pydata.org/community/blog/pandas-3.0.html) Support
  - Updated the pandas dependency to allow pandas 3.x
  - Added a compatibility shim for NDBC missing-value handling related to pandas 3 object to String dtype api changes
  - Fixed a deprecated period alias in tests
  - #443
  - Author: @simmsa
  - Reviewer: @akeeste
  - Fixes: #440
- Updated GitHub Actions CI, expanded installation/developer documentation, refreshed dev environment, and trimmed dependencies
  - Refactored optional dependencies
  - Standardized conda/conda-forge environment builds
  - Scoped black linting to changed files
  - #436
  - Author: @simmsa
  - Reviewer: @akeeste
- Update `rex` dependency to target pypi package to [`NLR-rex[hsds]>=0.5.0`](https://pypi.org/project/NLR-rex/)
  - #449
  - Author: @simmsa
  - Reviewer: @akeeste
@simmsa simmsa mentioned this pull request Jul 8, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akeeste akeeste akeeste approved these changes

Assignees

@akeeste akeeste

Labels

river module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@browniea @akeeste @simmsa