Skip to content

Conversation

@degoeden
Copy link
Contributor

I know this is already possible using the multiple run condition work flow, but for my work flow that process seemed inconvenient and not very intuitive. See discussion #1410 for more background.

This PR has a small edit to the BodyClass constructor, allowing for either an h5File to be the input (so the old workflow shouldn't be disrupted), or a structure. I've included a quick check on the structure format (mostly just to make sure the structure that BEMIO uses for writing the h5file isn't mistakenly being used) but it might be worth it to add a more robust check.

Additionally I made an edit to the initializeWecSim script to make use of the new capability this constructor enables.

Let me know if you think adding a more detailed check on the structure is worthwhile and if any additional documentation would be helpful.

MShabara and others added 11 commits January 6, 2025 13:58
* fix on pDis function call

* preliminary fix for issue WEC-Sim#1288

* Update readAQWA.m (WEC-Sim#1253)

* body mask fixes for WEC-Sim#1346

* add warning when variable hydro turned on for drag or nonhydro bodies

* flex body - add missing mask lines, define h5 button callback

* fix WEC-Sim#1288 cleanup waveClass

* adding bodyClass, should match

---------

Co-authored-by: jtgrasb <87095491+jtgrasb@users.noreply.github.com>
Co-authored-by: akeeste <akeeste@sandia.gov>
* Fix irregular wave elevation with multiple directions

* Updates the Ubuntu and Matlab versions (WEC-Sim#1420)

* Updates the Ubuntu and Matlab versions

---------

Co-authored-by: Mohamed Shabara <84589678+MShabara@users.noreply.github.com>
direct hydro struct into dev branch
Small edits to improve usability
@kmruehl kmruehl added the Body Class Body Class (bodyClass.m) label Mar 1, 2025
@kmruehl kmruehl requested a review from akeeste March 1, 2025 05:28
@kmruehl
Copy link
Collaborator

kmruehl commented Mar 1, 2025

@degoeden thank you! @akeeste can you review this PR since you've been working with the hydro structure a lot lately (for variable hydro).

@akeeste
Copy link
Contributor

akeeste commented Mar 5, 2025

Thanks @degoeden I'll review this once we've resolved a few other outstanding items related to the body class

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.

@degoeden Thanks for adding this feature. And my apologies for the delay while I caught up on some other changes to the bodyClass.m.

I pushed a few minor updates to your branch:

  • renamed the input to the bodyClass constructor to prevent confusion with body.hydroData
  • moved the checks on body.hydroStruct to body.checkInputs()

The two things I'd like to do before merging are:

  1. reduce redundancy in the information that the bodyClass contains. Right now, if hydroData is input to the bodyClass constructor, it fills up the new variable body.hydroStruct. I tried to make that constructor input define body.hydroData directly, but don't want to have alternate order of operations during the body's pre-processing.

I think the best option is to clear body.hydroStruct after it is used to define body.hydroData in initializeWecSim.

  1. add a paragraph to the user manual, perhaps a new subsection below https://wec-sim.github.io/WEC-Sim/main/user/advanced_features.html#variable-hydrodynamics, briefly describing how and why to use this alternate body instantiation.

@akeeste
Copy link
Contributor

akeeste commented Mar 19, 2025

@degoeden I finished these last two steps. Let me know if this updated implementation still meets your needs.

And one more question--do you have a script that converts the BEMIO hydro structure to the bodyClass hydroData format? That would be a convenient helper function to include in the source for users of this workflow.

@degoeden
Copy link
Contributor Author

@akeeste yes, this still works with my current workflow. I built a function to do that linked here: https://github.com/symbiotic-engineering/GILL/blob/main/src/rebuildhydrostruct.m I can add it to the PR if that would be helpful, but please note I have not confirmed it would work for other applications than the one I'm currently using it for. I'd expect it would still work properly, but haven't confirmed it.

@akeeste
Copy link
Contributor

akeeste commented Mar 20, 2025

@akeeste yes, this still works with my current workflow. I built a function to do that linked here: https://github.com/symbiotic-engineering/GILL/blob/main/src/rebuildhydrostruct.m I can add it to the PR if that would be helpful, but please note I have not confirmed it would work for other applications than the one I'm currently using it for. I'd expect it would still work properly, but haven't confirmed it.

Yes that function would be very helpful for users. Please add it and I will review. If needed I will expand it to cover other WEC-Sim applications.

@akeeste
Copy link
Contributor

akeeste commented Mar 24, 2025

@degoeden I expanded your function to robustly cover other WEC-Sim features (multiple bodies, GBM, mean drift, etc). I tested it across all of the Applications .h5 files and it seems to be working well. There's a brief documentation section.

Let me know if this updated function (note there are a couple other input parameters) still works for you! If so, once the tests pass I will merge this PR.

@akeeste akeeste self-requested a review March 24, 2025 20:54
@degoeden
Copy link
Contributor Author

Yes, this is all working for me. I had to change the shape of the array I was passing into the hydro.cg and hydro.cb variables for the BEMIO structure, but I'm guessing that I was doing that wrong originally but getting away with it since I only have one body. Used to be (1,3) now I'm using (3,1).

@akeeste
Copy link
Contributor

akeeste commented Mar 25, 2025

Yes, this is all working for me. I had to change the shape of the array I was passing into the hydro.cg and hydro.cb variables for the BEMIO structure, but I'm guessing that I was doing that wrong originally but getting away with it since I only have one body. Used to be (1,3) now I'm using (3,1).

Great to hear. Agreed about the cg and cb--if there's just 1 body then it's probably indexed ambiguously at some point later on. Thanks for this development effort and let us know if you have any issues with it in the future.

@akeeste akeeste merged commit b25c833 into WEC-Sim:dev Mar 25, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Body Class Body Class (bodyClass.m)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants