Skip to content
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

Merged

Conversation

bjonkman
Copy link
Contributor

Feature or improvement description

  • merged OpenFAST/dev branch
  • merged OpenFAST/3.5.1 branch
  • fixed merge conflicts: There were many, so an additional set of eyes on these changes would be good!
  • fixed some uninitialized output variables in an AeroDyn subroutine
  • fixed some FlowField issues with Visual Studio build
  • fixed some issues in steady-state and linearization code caused from introduction of FlowField

Impacted areas of the software
All

Additional supporting information
Things that I have noticed still need work:

  1. AeroDyn still treats InflowOn* variables as inputs, though they are not module inputs any more.
  2. AeroDyn's FlowField parameter is set in the OpenFAST glue code and the ADI module (for the AeroDyn driver) instead of using an InitInputType variable.
    • If InflowWind or ExternalInflow are not used in OpenFAST, this parameter is null.
    • Steady-state solver sets a constant wind speed. I've made a work-around so it avoids FlowField in this case, but when (1) is fixed, this will no longer work.
  3. Linearization with InflowWind and/or AeroDyn is not working. I have updated some code so that the cases run to completion, but the results need to be looked at more carefully. At a minimum, the matrices have different dimensions. The mapping in the glue-code linearizaton code may not work either, since InflowWind's matrices don't have the same sizes they used to (most of the inputs are missing).

Test results, if applicable
The linearization tests with AeroDyn fail. Other tests are now working.

reos-rcrozier and others added 30 commits January 19, 2023 22:28
…date

Update WAMIT .hst input file and baselines for floating MHK r-test
…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!
[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....
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.
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
Copy link
Collaborator

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.

Copy link
Contributor Author

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.

Copy link
Contributor

@ebranlard ebranlard Sep 15, 2023

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)
Copy link
Collaborator

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?

Copy link
Contributor Author

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.

Copy link
Contributor

@ebranlard ebranlard Sep 15, 2023

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.

@deslaughter
Copy link
Collaborator

deslaughter commented Sep 15, 2023 via email

@andrew-platt
Copy link
Collaborator

andrew-platt commented Sep 27, 2023

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?

@bjonkman
Copy link
Contributor Author

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

  • ef1267f to fix a bug and
  • 91f7bfd to fix uninitialized output variables.

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?

@andrew-platt
Copy link
Collaborator

I was getting mixed up in comparing rc-3.5.1 to dev to dev-unstable-pointers and lost track of where the CompAeroMaps was introduced. I now see that was in a PR to dev.

Everything looks good to me!

@andrew-platt
Copy link
Collaborator

One item to note on this PR: we decided to disable the Fake5MW_AeroLin_B3_UA6 test case temporarily. This will be revisited before the 4.0 release.

@andrew-platt andrew-platt merged commit 7091e85 into OpenFAST:dev-unstable-pointers Sep 28, 2023
21 checks passed
@bjonkman bjonkman deleted the dev-unstable-pointers branch September 28, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants