-
Notifications
You must be signed in to change notification settings - Fork 184
build Body from hydro structure directly #1421
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
Conversation
* 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
|
Thanks @degoeden I'll review this once we've resolved a few other outstanding items related to the body class |
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.
@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.hydroStructtobody.checkInputs()
The two things I'd like to do before merging are:
- reduce redundancy in the information that the bodyClass contains. Right now, if
hydroDatais input to the bodyClass constructor, it fills up the new variablebody.hydroStruct. I tried to make that constructor input definebody.hydroDatadirectly, 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.
- 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.
|
@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 |
|
@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. |
|
@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. |
|
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. |
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.