Skip to content

NDBC Updates#437

Open
jmcvey3 wants to merge 3 commits intoMHKiT-Software:developfrom
jmcvey3:ndbc-updates
Open

NDBC Updates#437
jmcvey3 wants to merge 3 commits intoMHKiT-Software:developfrom
jmcvey3:ndbc-updates

Conversation

@jmcvey3
Copy link
Copy Markdown
Contributor

@jmcvey3 jmcvey3 commented Mar 19, 2026

Opening this to solve Issue #427

I changed the source code to return degrees instead of radians, which was a simple fix. I also went ahead and updated the polar plots so that zero degrees is at the top of the figure, and positive degrees runs clockwise. We should update this in other polar plots in MHKiT too.

I also updated units to pass CF conventions, whereafter I noticed that our tests for this code check that the units are the same as what NDBC outputs. I'm not sure that's necessary. I also saw that we're not actually testing the value output from the source code, which is important.

@jmcvey3 jmcvey3 requested review from akeeste and simmsa March 19, 2026 18:57
Copy link
Copy Markdown
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 for getting to this @jmcvey3. Approved, no changes from me.

Resolves issue #427

@akeeste
Copy link
Copy Markdown
Contributor

akeeste commented Apr 2, 2026

Tests are failing for other reasons and as far as I can tell the changes to the calculations here wouldn't be caught by them anyways. Our NDBC test for create_directional_spectrum and create_spread_function only check the size and units of the outputs but no numerical values

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 this pull request may close these issues.

2 participants