Skip to content

New features - #18

Open
alliemr123 wants to merge 17 commits into
Project-SEA-Stack:devfrom
alliemr123:main
Open

alliemr123 wants to merge 17 commits into
Project-SEA-Stack:devfrom
alliemr123:main

Conversation

@alliemr123

Copy link
Copy Markdown
Collaborator

Adding four files: H5 output reader, corresponding readme description, MHKiT wave yaml writer, and corresponding description.

@alliemr123 alliemr123 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

added --direction_file for custom input wave directions

@alliemr123 alliemr123 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added --save feature to save .pngs of figures

@alliemr123 alliemr123 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fix formatting with yaml.dump on body damping

@alliemr123 alliemr123 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated directional writing/formatting for WITT model

@dav-og

dav-og commented Oct 23, 2026

Copy link
Copy Markdown
Member

Thanks Allie - this looks really useful. I've had a local run through the writer/reader and the overall approach looks good. Keeping each tool as one script plus a README is the right shape. There are a few things I think we should fix or clarify before merging:

  • The hard-coded NREL certificate path in MHKiT_writer.py points at a file on your machine, so NDBC downloads fail for everyone else. Please remove those lines. Anyone who needs a custom CA bundle can set REQUESTS_CA_BUNDLE themselves.
  • The tools are currently under data/demos/features/, which is outside the tree that gets packaged. CMakeLists.txt only installs data/demos/run_seastack/. Please move them to data/demos/run_seastack/features/, which is the location the READMEs already describe (writer, reader). dev is now up to date with main, so please merge dev into this branch first. The package check will then be in your tree, and it still expects h5outputsReader.py at the top of demos/ (check_package_contents.py), so that line needs updating too.
  • The README paths and examples need a quick tidy-up. A few commands use -buoy rather than --buoy (for example here). Please add a requirements.txt next to the scripts (mhkit[wave] for the writer; h5py, numpy, and matplotlib for the reader) and mention it in both READMEs. The screenshots are hosted on GitHub, so a short note that the figures are on the GitHub copy would help anyone reading the README from a clone or the release ZIP.
  • Once dev is merged in, two short pointers: a "Demo tools" section in data/demos/run_seastack/README.md, and a demos/features/ row in docs/build/PACKAGE_LAYOUT.md that names the MHKiT dependency. No separate theory or extending page.

A couple of things on the writer behaviour:

  • The regular-wave path writes height from the mean Hm0 (Hs_rep, used here). SEA-Stack treats height as crest-to-trough, so that wave carries about twice the energy of the measured sea state. Worth calling out in the README, and worth thinking about whether the written height should be the energy-equivalent value instead.
  • If depth is absent from the YAML, the writer inserts the buoy depth (_resolve_depth). Is that deliberate? I'm slightly wary of changing water depth implicitly, since it needs to stay consistent with the hydrodynamic and MoorDyn data.

One smaller thing: rewriting the hydro YAML through PyYAML strips existing comments (the usual write, and the same pattern at line 662 and line 2374). It would be nice to preserve those if practical. Wrapping the work in a main() guard would also help: importing the writer currently parses arguments and can download data or edit a YAML.

Nothing here looks fundamental, and I didn't hit any conflicts with current dev or the NL FK work. Once those points are addressed I think this should be in good shape.

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