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

Remove QuKi (quad-precision) from OpenFAST. #1453

Merged
merged 4 commits into from
Feb 28, 2023

Conversation

deslaughter
Copy link
Collaborator

This pull request is ready to be merged.

Feature or improvement description

The changes in this pull request remove the quad-precision (QuKi) from OpenFAST. When the DOUBLE_PRECISION CMake option was set to ON, it caused ReKi reals to be promoted to real64 precision (double) and DbKi reals to be promotoed to real128 (quadruple precision). Some architectures and compilers do not support quadruple precision (Flang) which caused issues with compiling. In addition, using quad-precision numbers is significantly slower because operations are typically implemented in software instead of in hardware. This change had negligible effects on the linearization regression test outputs (minor fluctuations in numbers near zero).

Related issue, if one exists

Impacted areas of the software

NWTC Library had the most modifications where all uses of QuKi and associated routines were removed.

  • SingPrec.f90
  • YAML.f90
  • JSON.f90
  • NWTC_Num.f90
  • NWTC_IO.f90
  • NWTC_RandomNumber.f90
  • NWTC_SLATEC.f90

HydroDyn Driver: changed dcm from R8Ki to ReKi to match main HydroDyn code and resolve issue with generic interface.

UnsteadyAero: discovered that the reference output in the unit test had uninitialized data in the first time step because it wrote several members of the KC (UA_KelvinChainType) before they had been set. Modified the registry file to provide initial values for all members of the structure (0.0).

Additional supporting information

Test results, if applicable

Updated unit test reference results for the UnsteadyAero_Driver. Branch created in r-test which will need to be merged into dev. All other tests pass.

This commit removes quad-precision kind, QuKi, from OpenFAST. As such:
- DbKi = real64
- R8Ki = real64
- SiKi = real32
- ReKi = real64 if DOUBLE_PRECISION else real32

Routines which used QuKi (R16) in generic interfaces have been removed.
The compiler flagss have been modified to set the default real
to 8 bytes and the default double to 8 bytes when DOUBLE_PRECISION
is on. This change should improve type clarity and remove the dependency
on software implemented quad-precision floating point numbers.

The UnsteadyAero unit test results were updated as the test
wrote uninitialized values to the output file. This has been corrected
by initializing members of the UA_KelvinChainType structure to 0.0.
bjonkman and others added 2 commits February 14, 2023 13:22
- change `DBLE` to REAL(*,R8Ki)
- remove QuKi from rest of the Sys*.f90 routines
- make default double 64 instead of 128 in VS project files DP configurations
- consistent variable types in `DCM_expR` and `DCM_SetLogMapForInterpR`and other routines in NWTC_Num.f90
More updates for new `DbKi=R8Ki` behavior
This fixes an issue in the UnsteadyAero regression test
@deslaughter deslaughter merged commit 59c11c1 into OpenFAST:dev Feb 28, 2023
@deslaughter deslaughter deleted the f/del-quki branch February 28, 2023 20:38
@@ -60,18 +57,18 @@ MODULE Precision
INTEGER, PARAMETER :: IntKi = B4Ki !< Default kind for integers
INTEGER, PARAMETER :: BYTES_IN_INT = 4 !< Number of bytes per IntKi number - use SIZEOF()

#if !defined (DOUBLE_PRECISION) && !defined (OPENFAST_DOUBLE_PRECISION)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this DOUBLE_PRECISION preprocessor definition check made the vs-build project files not able to build in double precision any more. (Notice that the new code checks only for OPENFAST_DOUBLE_PRECISION). I am adding a fix for this in #1008, but maybe we need to update documentation, too? See the original reason for the two preprocessor definitions here: #524

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deslaughter, can you take a look at this? I think we need to fix it for 3.5.0

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bjonkman, could I change the vs-build project files to define OPENFAST_DOUBLE_PRECISION instead of DOUBLE_PRECISION?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deslaughter You can cherrypick this commit with the changes to the vs-build files: 9787e50 Hopefully I got them all.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bjonkman, thanks for creating the commit. I've cherry-picked it into a bugfix branch and created PR #1560. I looked through the VS project files and updated DOUBLE_PRECISION in AeroDyn_Inflow_c_binding.vfproj.

bjonkman added a commit to bjonkman/openfast that referenced this pull request May 8, 2023
…SION`

This backward-compatibility of preprocessor definitions from OpenFAST#524 was broken in OpenFAST#1453.
deslaughter pushed a commit to deslaughter/openfast that referenced this pull request May 8, 2023
…SION`

This backward-compatibility of preprocessor definitions from OpenFAST#524 was broken in OpenFAST#1453.
deslaughter added a commit to deslaughter/openfast that referenced this pull request May 8, 2023
…SION`

This backward-compatibility of preprocessor definitions from OpenFAST#524 was broken in OpenFAST#1453.
deslaughter added a commit to deslaughter/openfast that referenced this pull request May 8, 2023
This backward-compatibility of preprocessor definitions from OpenFAST#524 was broken in OpenFAST#1453.
@andrew-platt andrew-platt mentioned this pull request May 12, 2023
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants