-
Notifications
You must be signed in to change notification settings - Fork 447
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
Rename FAST to OpenFAST in source #116
Rename FAST to OpenFAST in source #116
Conversation
Please see the regression test results below for this pull request. Of note is the there is an existing bug in the BeamDyn unit tests,
|
@jjonkman @ghaymanNREL @michaelasprague Please let me know if you have any feedback regarding the updated naming that I've added here. I chose to avoid renaming every instance of "fast" to "openfast" in order to prevent any unnecessary bugs since modules and subroutine names contain "fast". Furthermore, I'd like to minimize introducing breaking changes for anyone using OpenFAST as a dll like @gantech. |
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.
The changes to NWTC_IO.f90/CheckArgs() are not universal. This routine is used not only by OpenFAST, but many other related programs including standalone drivers, TurbSim, etc.
The first problem is that "IF ( NumArg .EQ. 0 ) THEN" has been added to display the syntax. But this message should not be displayed if the program accepts a DefaultInputFile.
The second problem is that statements have been added regarding restart/checkpoint files, but these statements only apply to OpenFAST, not to other programs that may use this SUBROUTINE.
You may need to make a custom CheckArgs routine for OpenFAST (but this then should probably not be included in the NWTC Subroutine Library).
Everything else in this pull request is OK.
Add restart file syntax and show it in these cases: - the help flag is given (h, H, ?) - no arguments are given
7a2e9ec
to
6f679e3
Compare
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.
Why were lines 2659-2661 removed from NWTC_IO.f90?
The other changes look fine with me.
This subroutine is generic and should support main programs which allow for a default input file. In that case, NumArgs is 0 but it is not an error.
6f679e3
to
b8bcbdd
Compare
@jjonkman that was a mistake, thank you for catching that. I've added those lines back in the latest commit. |
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.
Thanks for addressing my comments.
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.
This looks good.
* rename glue-codes/fast to glue-codes/openfast * update gitiginore * remove the version from fast registry * Rename CMake utilities * Rename the cpp glue code * rename fast registry to openfast registry * rename fast-library to openfast-library * Update r-test submodule and add new beamdyn specific test * Documentation updates for fast to openfast name change * Bug fix in displaying the program name * Get naming updates from the r-test submodule
This pull request selectively renames occurrences of "FAST" to "OpenFAST" in order to more accurately reflect the transition from FAST 8.16 to OpenFAST v1.0.0.
The following occurrences have been renamed
The following occurrences have not been renamed
modules-ext
The code in pull request #107 have been incorporated here, so thanks to @ NobuhiroKUSUNO for the help. Also, this pull request is a dependency of #105.