Consistent double precision definitions

[islas]

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) :

#if ( RWORDSIZE == 8 )
#if ( RWORDSIZE == DWORDSIZE )
#if ( DWORDSIZE == 8 && RWORDSIZE == 8 )

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 :

 grep -RiE "WRF_REAL.*=.*[0-9]+" | sort -u
# original version
external/ioapi_share/wrf_io_flags.h:      integer, parameter  :: WRF_REAL                             = 104
external/io_grib1/io_grib1.f90:      integer, parameter  :: WRF_REAL                             = 104
external/io_grib_share/io_grib_share.f90:      integer, parameter  :: WRF_REAL                             = 104
external/io_int/diffwrf.f:      integer, parameter  :: WRF_REAL                             = 104
external/io_int/io_int.f:      integer, parameter  :: WRF_REAL                             = 105
external/io_netcdf/wrf_io.f:      integer, parameter  :: WRF_REAL                             = 104
frame/module_io.f90:      integer, parameter  :: WRF_REAL                             = 105
frame/module_quilt_outbuf_ops.f90:      integer, parameter  :: WRF_REAL                             = 105
# modified version
# inc/wrf_io_flags.h:      integer, parameter  :: WRF_REAL                             = 105
main/real_em.f90:      integer, parameter  :: WRF_REAL                             = 105
share/input_wrf.f90:      integer, parameter  :: WRF_REAL                             = 105
share/mediation_integrate.f90:      integer, parameter  :: WRF_REAL                             = 105
share/output_wrf.f90:      integer, parameter  :: WRF_REAL                             = 105
share/track_input.f90:      integer, parameter  :: WRF_REAL                             = 105
share/wrf_bdyin.f90:      integer, parameter  :: WRF_REAL                             = 105
share/wrf_bdyout.f90:      integer, parameter  :: WRF_REAL                             = 105
share/wrf_tsin.f90:      integer, parameter  :: WRF_REAL                             = 105
var/da/da_main/da_update_firstguess.inc:  wrf_real=104

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 the sed command in external/ioapi_share/makefile where wrf_io_flags.h is changed in the inc/ folder only, and thus anything including external/ioapi_share first has one definition whilst anything including inc/ 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 use 104 or 105 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 :

#ifdef PROMOTE_FLOAT
  integer, parameter :: WRF_FLOAT = WRF_DOUBLE
#else
  integer, parameter :: WRF_FLOAT = WRF_REAL
#endif

The end result will always be WRF_FLOAT = WRF_REAL regardless of -r8 option since PROMOTE_FLOAT is not defined anywhere in the configuration / compilation logic. However, WRF_FLOAT happens to be used correctly since the sed rewrite has changed WRF_REAL to 105 (effectively the same as WRF_FLOAT = WRF_DOUBLE). This only works because WRF_FLOAT is exclusively used only in files that access the inc/wrf_io_flags.h rewritten file and not the external/ioapi_share/wrf_io_flags.h one. Furthermore, aside from io_int.F, no code that contains the 105 value utilizes WRF_REAL

# Looking for any incorrect usage of WRF_FLOAT in files with 104 value
# we're really only concerned with unique statements of computational code
grep -RiEl "WRF_REAL.*=.*104" | sort -u  | xargs -i grep -iH WRF_FLOAT {} | sort -u
external/ioapi_share/wrf_io_flags.h:      integer, parameter  :: WRF_FLOAT=WRF_DOUBLE
external/io_grib1/io_grib1.f90:      integer, parameter  :: WRF_FLOAT=WRF_REAL
external/io_grib_share/io_grib_share.f90:      integer, parameter  :: WRF_FLOAT=WRF_REAL
external/io_int/diffwrf.f:      integer, parameter  :: WRF_FLOAT=WRF_REAL
external/io_netcdf/wrf_io.f:      integer, parameter  :: WRF_FLOAT=WRF_REAL
# No usage of bad value, only the include declaration shows up

# Look for usage of WRF_REAL where its value has been changed to 105 thus
# leading to ambiguous definitions
# Exclude declarations for brevity 
grep -RiEl "WRF_REAL.*=.*105" | sort -u  | xargs -i grep -iH WRF_REAL {} | sort -u | grep -vE "integer, parameter[ ]*:: WRF_"
external/io_int/io_int.f:            CALL wrf_message('io_int.F90: ext_int_read_field: types other than WRF_REAL not supported yet')
external/io_int/io_int.f:  IF      ( FieldType .EQ. WRF_REAL .OR. FieldType .EQ. WRF_DOUBLE) THEN
external/io_int/io_int.f:          IF      ( FieldType .EQ. WRF_REAL ) THEN
external/io_int/io_int.f:    IF      ( FieldType .EQ. WRF_REAL ) THEN
# These are character strings
main/real_em.f90:      IF ( config_flags%auxinput1_inname(1:8) .NE. 'wrf_real' ) THEN
share/input_wrf.f90:              ( config_flags%auxinput1_inname(1:8) .EQ. 'wrf_real' ) ) ) THEN

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 or 8 for single or double precision, respectively, where those are already defined by NATIVE_RWORDSIZE and DWORDSIZE.

• 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 and DOUBLE_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 have DOUBLE_PRECISION control an rkind setting RWORDSIZE much like kind_phys for ccpp

For areas where WRF_REAL was used with a value of 105 when -r8 is used (io_int.F), to maintain previous behavior the value should be changed to WRF_FLOAT. Instead of using sed to rewrite the file, #ifdef PROMOTE_FLOAT will use the valid DOUBLE_PRECISION define to switch control of WRF_FLOAT to WRF_DOUBLE if defined, or WRF_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 to inc/, this may be addressed separately.

TESTS CONDUCTED:

1. Single and double precision builds should compile without issue and have effectively the same logic as before

[islas]

Requires #2056, #2053, #2086, #2087, and #2088

[weiwangncar]

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

[amstokely]

All of these changes look good to me.