[BugFix] change time handling to double precision for G4D timestep index #1684
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
The time handling for G4D time index finding was a mix of single and double precision. This could lead to finding the wrong index in a G4D wind grid passed from FF to an OpenFAST turbine, in which case the index might be outside the G4D box that was passed. We think this can occur with a long simulation of 4,200 s, with an OpenFAST DT of 0.0378 s, which resulted in a time index out of bounds at time 4132 seconds.
There was a bug where the modulo operation to wrap back to the beginning of the wind file didn't work correctly, previously noted in issue OpenFAST#471. This seemed to come back due to some changes in the IfW refactor. The ifw_HAWC regression test was added to catch this and other IfW changes in the future.
This commit specifies Python_ROOT_DIR for each github action since each Job may have a different version of Python and a different path to the executable. By specifying Python_ROOT_DIR each Job should get a valid path.
deslaughter
approved these changes
Jul 17, 2023
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 PR is ready for merging, pending testing from @MYMahfouz (see #1615 (reply in thread)).
Feature or improvement description
The time handling for G4D time index finding was a mix of single and double precision. This could lead to finding the wrong index in a G4D wind grid passed from FF to an OpenFAST turbine, in which case the index might be outside the G4D box that was passed. We think this can occur with a long simulation of 4,200 s, with an OpenFAST DT of 0.0378 s, which resulted in a time index out of bounds at time 4132 seconds.
Related issue, if one exists
#1615 (comment)
Impacted areas of the software
This only affects long simulations with FAST.Farm.
Test results, if applicable
No test cases are affected (we don't run any of them that long).