Cleanup HydroDyn and SeaState code #1864
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- removed PointsToSeaState (still need to fix some more pointers, though) - removed InitInp PtfmLocationX and Y from HD since it's used in SeaState - cleaned up some VS projects related to SeaSt/HD
This is already stored in the WaveField type, so doesn't need extra copies.
also added subroutines to add 1st and 2nd order arrays without copying all the error checks
These are now stored in WaveField data since the modules that need them already access the rest of the WaveField data. Also, all of the SeaSt InitOut data that are sent to HD Init are now done in one place instead of two (originally there was some issue with pointers in the SeaSt InitOut type, but that is not a concern any more).
- also removed extra pointer for WaveTime in SS Excitation module - cleaned up some text handling in SS Excitation (some incorrect routine names in error message and a lot of extra repeated string manipulation) - Do we have a test for SS-Excitation?
- `LastIndWave` in HD - `DT` in SeaSt
andrew-platt
added
Module: HydroDyn
Module: SeaState
|
@luwang00, can you also review? |
|
As I was updating some of these variables, I question whether some of the tests involving Also, I think someone should double check where See comments with |
luwang00
reviewed
Nov 9, 2023
luwang00
reviewed
Nov 9, 2023
luwang00
reviewed
Nov 9, 2023
luwang00
reviewed
Nov 9, 2023
luwang00
reviewed
Nov 9, 2023
deslaughter
approved these changes
Nov 15, 2023
bjonkman
commented
Nov 16, 2023
bjonkman
commented
Nov 16, 2023
- removed a bunch of `bjj` comments in the code - simplified some of the checks based on WaveMod (made them easier to read by specifying exactly which models the checks apply to) - updated default z depth using entered water depth instead of the default water depth - removed override of `Hs` when it's not used - override WvHiCOff and WvLowCOff in the cases where they are not supposed to be used; the code DOES actually use them, so they need to be set to allow all frequencies in the cases where they are not supposed to be used. - put check on valid values of WaveStMod from HydroDyn into SeaState (and removed the conflicting SeaState check)
andrew-platt
approved these changes
Nov 17, 2023
andrew-platt
merged commit 897d6ee
into
OpenFAST:dev-unstable-pointers
21 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feature or improvement description
This PR removes duplicate copies of data and pointers of many variables shared between SeaState and HydroDyn. These shared data are now stored in the
WaveFielddata type (stored as a SeaState parameter).It also removes duplicated error checks, outdated code (including outdated driver codes for some HydroDyn submodules), and outdated comments. Hopefully these changes will make it easier to read the code, though more updates could be done.
Some particular changes to note:
WtrDpthhas been replaced withEffWtrDpthin places where it had addedMSL2SWLto the variable.WtrDpthis now ALWAYS the value from the input file. This should help remove confusion about whether that variable is being used before or after addingMSL2SWL.Impacted areas of the software
HydroDyn and SeaState; glue codes that connect the two modules are also modified.
Test results, if applicable
Regression test results have not changed.
Someone should add a test where
MSL2SWLis non-zero, though. We should also add tests for the new features that have been added recently.