Open islas opened 1 month ago
Requires #2056, #2053, #2086, #2087, and #2088
The regression test results:
Test Type | Expected | Received | Failed
= = = = = = = = = = = = = = = = = = = = = = = = = = = =
Number of Tests : 23 24
Number of Builds : 60 57
Number of Simulations : 158 150 0
Number of Comparisons : 95 86 0
Failed Simulations are:
None
Which comparisons are not bit-for-bit:
None
TYPE: bug fix
KEYWORDS: double precision, configuration, make, cmake
SOURCE: internal
DESCRIPTION OF CHANGES: Problem: Currently, the source code has multiple preprocessor definitions for controlling double precision usage (1). Likewise, there are multiple parameter definitions in the IO code for the
WRF_REAL
value (2).Examples of (1) :
Because there is no definitive define for querying double precision, it has been left as an exercise to the contributor to formulate an adequate conditional. While the above and other such forms work, they are not consistent and can be confusing as to the intent.
Examples of (2) in a directory with option
-r8
:Across many different preprocessed files, there appears to be two values of
WRF_REAL
which could lead to undesired behavior when interfacing between different sections of code. This issue arises from thesed
command inexternal/ioapi_share/makefile
wherewrf_io_flags.h
is changed in theinc/
folder only, and thus anything includingexternal/ioapi_share
first has one definition whilst anything includinginc/
has the changed value.While (2) may seem like an entirely separate problem from (1) they are interrelated.
wrf_io_flags.h
already partially contains the necessary logic to control whether to use104
or105
when double precision promotion is requested. The current logic is not being used correctly fully as it uses a totally different (and undefined) form of double precision query :The end result will always be
WRF_FLOAT = WRF_REAL
regardless of-r8
option sincePROMOTE_FLOAT
is not defined anywhere in the configuration / compilation logic. However,WRF_FLOAT
happens to be used correctly since thesed
rewrite has changedWRF_REAL
to105
(effectively the same asWRF_FLOAT = WRF_DOUBLE
). This only works becauseWRF_FLOAT
is exclusively used only in files that access theinc/wrf_io_flags.h
rewritten file and not theexternal/ioapi_share/wrf_io_flags.h
one. Furthermore, aside fromio_int.F
, no code that contains the105
value utilizesWRF_REAL
Solution: To reduce the overall complexity of various define constructs and IO inconsistencies a singular define
DOUBLE_PRECISION
can be introduced specifically meant to inform sections of code whether double precision promotion has been requested.Adding "one more define" may not sound appealing at first, but it does carry some benefits :
Firstly, it does not require the user to hard code values of
4
or8
for single or double precision, respectively, where those are already defined byNATIVE_RWORDSIZE
andDWORDSIZE
.Furthermore, rather than use
#if (RWORDSIZE == DWORDSIZE)
logic can be simplified to a more coherent statement of#ifdef DOUBLE_PRECISION
which is more readable and better understood in intent.Finally, reducing duplication of defines to do the same thing is good practice and limiting defines to control one thing only avoiding co-opting for multiple roles is also good practice. The first part sounds counter to a new define but
RWORDSIZE
andDOUBLE_PRECISION
serve two different functions - the former to tell us the numeric size of real data in bytes and the latter to tell us if alternate logic should be used. An alternative solution to only use a single define would be to haveDOUBLE_PRECISION
control anrkind
settingRWORDSIZE
much likekind_phys
for ccppFor areas where
WRF_REAL
was used with a value of105
when-r8
is used (io_int.F
), to maintain previous behavior the value should be changed toWRF_FLOAT
. Instead of usingsed
to rewrite the file,#ifdef PROMOTE_FLOAT
will use the validDOUBLE_PRECISION
define to switch control ofWRF_FLOAT
toWRF_DOUBLE
if defined, orWRF_REAL
if not.For the CMake build,
DOUBLE_PRECISION
needs to be added to the defines. No other changes necessary.To reduce the complexity of these changes,
wrf_io_flags.h
is still copied out toinc/
, this may be addressed separately.TESTS CONDUCTED: