SeaState: combine SeaSt_Interp into SeaSt_WaveField, and bug fix#2026
Merged
andrew-platt merged 8 commits intoOpenFAST:dev-unstable-pointersfrom Feb 8, 2024
Merged
Conversation
There is no reason to ever acces the interp routines directly outside of the WaveField module, so combining the two.
Also use this routine internally, and condense a bunch of redundant code sections as a result
Attempting to troubleshoot the issue led to cleaning up when I couldn't really follow what was going on due to too much error hanlding in the way.
andrew-platt
commented
Feb 8, 2024
bjonkman
reviewed
Feb 8, 2024
bjonkman
reviewed
Feb 8, 2024
Contributor
|
@andrew-platt I confirm that this PR runs with my OpenFAST model (Morison-only approach) and returns the expected results: |
Add interface WAMIT_ForceWaves_Interp (WAMIT_Interp) for * WAMIT_ForceWaves_Interp_3D_vec6 -- 3D interpolation * WAMIT_ForceWaves_Interp_4D_vec6 -- 4D interpolation Also added WaveField_Interp_4D_Vec6 routine to SeaSt_WaveField for completeness (not currently used). Also an update to the RunRegistry.bat file for something missed before.
This was excess code that simply isn't needed.
deslaughter
approved these changes
Feb 8, 2024
Contributor
|
Thanks @andrew-platt for the bug fix and the code cleanup. Looks great to me. |
This file contains hidden or 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
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.

This is ready for merging
@luwang00, please review.
@RBergua, please check that this now works for your model.
Feature or improvement description
There was a bug in the how the indices for time in the wave field data was calculated when
deltaTwas zero. This resulted in nonsensical bounds for time, which caused a segmentation fault during interpolation.Included in this PR:
SeaState_Interphas been combined into theSeaSt_WaveFieldmodule. While attempting to find the root issue, it became obvious that there was no reason to keep these separated. This includes revisions to how the misc vars and parameters are stored for WaveField.SeaState_Interproutines. New interfaces were provided withinSeaSt_WaveFieldso that the WaveField discretization information could be used in HydroDyn to find the indices and weighting during interpolation (HydroDyn shares the same grid discetization for the WAMIT forces).Waves.f90was revised with some updated error handling so that it is more readable. I couldn't figure out what was going on before cleaning and formatting a bit.SeaState.f90were converted to WaveField function calls instead.deltaTwas added to the interpolation functions.Related issue, if one exists
@RBergua ran into a segmentation fault with the HydroDyn driver when running linearization from the driver only. The model is proprietary so it cannot be shared.
Impacted areas of the software
HydroDyn and SeaState
Additional supporting information
Test results, if applicable
A new HD driver test should be added to exercise the driver linearization function.