-
Notifications
You must be signed in to change notification settings - Fork 458
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
Merge dev and 3.5.1; some bug fixes #1767
Merge dev and 3.5.1; some bug fixes #1767
Conversation
This reverts commit 5657f25.
…date Update WAMIT .hst input file and baselines for floating MHK r-test
Remove TurbineType parameter
…date Update HydroDyn input file and baselines for floating MHK r-test
The input arrays for Lidar to ServoDyn get allocated in ServoDyn.f90, and again in BladedInterface_EX.f90. This double allocation causes issues for many compilers. The logic behind all this really should be looked at in more detail, but since the Lidar module in InflowWind will be superceeded by LidarSim in release 4.0.0, I'm not going to sort the logic right now. Affected arrays: u%LidSpeed u%MsrPositionsX u%MsrPositionsY u%MsrPositionsZ Thanks to @foxton9 for reporting the issue, and proposing the fix!
This was reported in issue OpenFAST#1484
[BugFix] ServoDyn inputs for Lidar allocated twice, and a few other minor issues
[BugFix] HD wave visualization with 2nd order waves
- I don't really like these parameters in NWTC Library, but they seem to be used in many different modules, so that's the easiest place for now - some of the IF statements had redundant ELSEIF instead of just ELSE, so I simplified them from (IF A ... ELSEIF NOT A...) to (IF A ... ELSE...)
In the past, the only way to build openfastlib as a shared library was to set -DBUILD_SHARED=ON. This library is now always built as shared but many users still set BUILD_SHARED which causes all module libraries to be created as shared which means that all must be dynamically loaded at run time. This commit changes the CMakeLists.txt files to build all module libs as STATIC regardless of BUILD_SHARED. This means that all module libraries will be statically linked into openfast, openfastlib and FAST.Farm which should improve performance and reduce complexity
ED bug fix from prev commit 903cf07
- If user requests too many nodal outputs, the max number of outputs will be generated instead of none (as done previously) - call generic LAPACK_GEMM routines instead of writing LAPACK_DGEMM - return after error on Init_MiscVars to prevent issues when writing summary file BD: Explicitly initialize new InitInp in driver
MHK: add parameters for readability
Looks like it got commited accidentally on a rebase command....
Make CMake module libs STATIC
Initial AeroMap changes for ElastoDyn and BeamDyn
This should finally fix the issue raised in PR#1702 and PR#1501. A second modulo was added because small negative values will cause the first modulo to return a value equal to NSteps which then causes the code to think the time is out of bounds.
Add additional modulo in IfW_FlowField
real(R8Ki), dimension(:,:,:,:), pointer :: R_wi ! Alias. Transformation from inertial to "WithoutSweepPitchTwist" or "orientationAnnulus". TODO: deprecate me. | ||
integer(Intki), dimension(:) , pointer :: W2B ! Alias. Index from Wing index to Blade | ||
|
||
! Alias to shorten notations |
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.
Might be better to use an associate statement instead of pointers for shortening the variable names to avoid any pointer overhead. Feel free to disregard.
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.
Yes. That change is from #1679, though.
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.
I'm not familiar with associate statements, feel free to replace if you think it does the same but better. I'm just generally in favor of shorter notations :) I've used pointers as aliases in several places in the code (if you look for "alias", you'll likely find them, I'm often the one responsible for those...).
As Bonnie mentioned, this is already in 3.5.1, we can change it later.
real(R8Ki), intent(in ) :: CT_1 !< CT(1), value at a=1 | ||
real(R8Ki), intent(out) :: c0, c1, c2 !< coefficients of the second order polynomial | ||
real(R8Ki) :: denom | ||
denom = (a_c**2 - 2._R8Ki*a_c + 1.0_R8Ki) |
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.
Does this need a check to ensure that denom
is nonzero?
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.
That is a good question.... This is from some Envision code that was merged earlier (and I'm not sure is actually used in the OpenFAST AeroDyn). I am not sure if a_c can ever be 1, but going backwards in the code, it looks like there are some other numerical checks that need to happen before it even gets to this routine.
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.
a_c should never 1, its value is typically 0.4, but there are some bounds on it (never above 0.5) so we should be fine.
It probably doesn't make a significant difference, I thought there may have
been another reason for using the pointers. Just for information, associate
was introduced in Fortran 2003 and is explained on page 12 of
https://wg5-fortran.org/N1601-N1650/N1648.pdf. Its purpose is to provide
shorter notation without the overhead of declaring additional variables.
…On Thu, Sep 14, 2023 at 9:04 PM E. Branlard ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In modules/aerodyn/src/AeroDyn_AllBldNdOuts_IO.f90
<#1767 (comment)>:
> real(ReKi) :: psi_hub ! Azimuth wrt hub
- real(ReKi) :: Vind_g(3) ! Induced velocity vector in global coordinates
- real(ReKi) :: Vind_s(3) ! Induced velocity vector in section coordinates (AeroDyn "x-y")
-
+ real(R8Ki), dimension(:,:,:,:), pointer :: R_li ! Alias. Transformation from inertial to local-polar to airfoil (3x3xnNodesxnBlades)
+ real(R8Ki), dimension(:,:,:,:), pointer :: R_wi ! Alias. Transformation from inertial to "WithoutSweepPitchTwist" or "orientationAnnulus". TODO: deprecate me.
+ integer(Intki), dimension(:) , pointer :: W2B ! Alias. Index from Wing index to Blade
+
+ ! Alias to shorten notations
I'm not familiar with associate statements, feel free to replace if you
think it does the same but better. I'm just generally in favor of shorter
notations :) I've used pointers as aliases in several places in the code
(if you look for "alias", you'll likely find them, I'm often the one
responsible for those...).
—
Reply to this email directly, view it on GitHub
<#1767 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABCSATIK65JRROJQWOXMA53X2OSQDANCNFSM6AAAAAA4VAZYCE>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
HD Bug Fix: AddF0 for multiple potential-flow bodies with NBodyMod!=1
Not destroying this local data could cause memory leaks on some systems.
This will free up more space for writing output files
I see some changes in AeroDyn.f90 that I can't find in either rc-3.5.1 or dev. Are there some AD15 changes that didn't get into any of the other branches yet, or are some of the differences I can't find just a result of code cleanup? |
The only changes I made to AeroDyn.f90--other than fixing some spacing--are in I think there were some changes in this branch (not in this PR) for the InflowWind/FlowField pointers that were made before my changes, though. Can you point to a specific line that you're questioning? |
I was getting mixed up in comparing Everything looks good to me! |
One item to note on this PR: we decided to disable the |
7091e85
into
OpenFAST:dev-unstable-pointers
Feature or improvement description
Impacted areas of the software
All
Additional supporting information
Things that I have noticed still need work:
InflowOn*
variables as inputs, though they are not module inputs any more.FlowField
parameter is set in the OpenFAST glue code and the ADI module (for the AeroDyn driver) instead of using anInitInputType
variable.Test results, if applicable
The linearization tests with AeroDyn fail. Other tests are now working.