CMake build for WW3 fails when compiling S2SW with -DDEBUG=ON

[DeniseWorthen]

Description

Compiling UWM app S2SW in debug mode fails with a conflict between compile flag -DDEBUG=ON used at the app level and an internal WW3 variable named DEBUG

To Reproduce:

Check out current develop branch. Compile S2SW in debug mode:

./compile.sh hera.intel '-DAPP=S2SW -DDEBUG=ON -DCCPP_SUITES=FV3_GFS_v16_coupled_p8' '' YES NO 2>&1 | tee compile.log

The compile log shows a compile failure

/apps/cmake/3.20.1/bin/cmake -E touch CICE-interface/CMakeFiles/cice.dir/CICE/icepack/columnphysics/icepack_fsd.F90.o.provides.build
/tmp/ifort8M9ebj.i90: remark #5415: Feature not yet implemented: check shape
/scratch2/NCEPDEV/climate/Denise.Worthen/WORK/ufs-weather-model/WW3/model/src/w3gsrumd.F90(626): error #5082: Syntax error, found INTEGER_CONSTANT '1' when expecting one of: <IDENTIFIER>
                              NCB, NNP, 1 ) RESULT(GSU)
----------------------------------------^
/scratch2/NCEPDEV/climate/Denise.Worthen/WORK/ufs-weather-model/WW3/model/src/w3gsrumd.F90(636): error #5082: Syntax error, found INTEGER_CONSTANT '1' when expecting one of: <IDENTIFIER> %FILL
      LOGICAL, INTENT(IN), OPTIONAL :: 1

The source code shows that the "1" is a replacement for the WW3 variable DEBUG

      FUNCTION W3GSUC_PTR_R4( IJG, LLG, ICLO, XG, YG, &
                              NCB, NNP, DEBUG ) RESULT(GSU)
!     Single precision pointer interface
      TYPE(T_GSU)         :: GSU
      LOGICAL, INTENT(IN) :: IJG
      LOGICAL, INTENT(IN) :: LLG
      INTEGER, INTENT(IN) :: ICLO
      REAL(4), POINTER    :: XG(:,:)
      REAL(4), POINTER    :: YG(:,:)
      INTEGER, INTENT(IN), OPTIONAL :: NCB
      INTEGER, INTENT(IN), OPTIONAL :: NNP
      LOGICAL, INTENT(IN), OPTIONAL :: DEBUG

[kgerheiser]

So, UFS adds add_definitions(-DDEBUG) here

ufs-weather-model/cmake/GNU.cmake

Line 12 in af1e026

add_definitions(-DDEBUG)

Just need a way to undo that in the WW3 layer. Or change the variable name.

[DeniseWorthen]

I did make a fix which changed the variable name. That worked, but I wasn't sure if that was the easiest route.

[kgerheiser]

There's the CMake function remove_definitions(-DDEBUG), which could be added into the top-most if statement in WW3 that checks if it's building as a subdirectory of UFS.

[JessicaMeixner-NOAA]

@kgerheiser I'm going to take over the testing for Matt on this. I might have just missed something in the chain, but if we add the remove_definitions(-DDEBUG) would we then need to add something else at the UFS level so that WW3 does in fact use debug flags if UFS is saying to use debug flags?

[kgerheiser]

I don't think any other changes are necessary.

If you add remove_definitions here:
https://github.com/NOAA-EMC/WW3/blob/3f2730426fc28a5246ae48280cfe98a25873180c/CMakeLists.txt#L15-L20

WW3 will still pick up CMAKE_BULD_TYPE, and it has its own debug flags which will be applied:

https://github.com/NOAA-EMC/WW3/blob/3f2730426fc28a5246ae48280cfe98a25873180c/model/src/CMakeLists.txt#L47

But you may want to clear CMAKE_Fortran_FLAGS_DEBUG and CMAKE_C_FLAGS_DEBUG like already is already done for the non-debug flags: https://github.com/NOAA-EMC/WW3/blob/3f2730426fc28a5246ae48280cfe98a25873180c/CMakeLists.txt#L15-L20

because WW3 has its own debug flags.

[DeniseWorthen]

I think we want a way to retain the UWM debug flags (for example -init=snan,arrays) when WW3 is compiled as part of UWM.

I don't understand CMake enough to really follow the logic you're suggesting above.

[JessicaMeixner-NOAA]

What would the right combination of the two be where WW3 has the model specific compile flags it needs (eg "-assume byterecl") while being responsive to the over-all coupled model build/flag decisions?

[kgerheiser]

@DeniseWorthen

It should be fine to keep the UFS debug flags as well. They'll be appended in addition to the native WW3 debug flags.

When you set the debug build, it sets CMAKE_BUILD_TYPE=Debug

ufs-weather-model/CMakeLists.txt

Line 97 in af1e026

set(CMAKE_BUILD_TYPE "Debug" CACHE STRING "Set type of build to Debug." FORCE)

which tells CMake to build with debug flags and uses the flags provided in the variable CMAKE_<lang>_FLAGS_DEBUG (lang = C or Fortran).

I was saying to clear the flags provided by UFS and only use the native WW3 debug flags, just to avoid any conflicts like when UFS sets -r8/-r4 which conflicts with the WW3 build so we clear them when WW3 is built as a submodule.

[kgerheiser]

@JessicaMeixner-NOAA WW3 uses -assume byterecl internally irrespective of what UFS provides (because CMAKE_Fortran_FLAGS/CMAKE_C_FLAGS are cleared). Are there other flags that UFS should pickup from UFS?

You can leave it as it currently is (and undo the -DDEBUG defintion), and let UFS provide some extra debug flags. WW3 will pick those up automatically from CMAKE_Fortran/C_FLAGS_DEBUG.

[JessicaMeixner-NOAA]

@kgerheiser that would be fine. I misunderstood a comment to be replacing instead of adding to. Adding to is great. The -assume byterecl is the most key//unique WW3 flag and would be my biggest cause of concern.

[kgerheiser]

I created a PR to @DeniseWorthen's mesh cap branch. DeniseWorthen/WW3#3

[DeniseWorthen]

Thanks for the quick fix. This seems to have worked; these are the arguments used to compile the source code w3wavemd.F90 for example:

-g -O0 -check -check noarg_temp_created -check nopointer -warn -warn noerrors -fp-stack-check -fstack-protector-all -fpe0 -debug -ftrapuv -init=snan,arrays -module mod -no-fma -ip -g -traceback -i4 -real-size 32 -fp-model precise -assume byterecl -convert big_endian -fno-alias -fno-fnalias -O0 -debug all -warn all -check all -check noarg_temp_created -fp-stack-check -heap-arrays -fpe0 -qopenmp -c

[kgerheiser]

Yeah, some duplicated flags, but that doesn't matter.

[JessicaMeixner-NOAA]

The fix from Kyle for this is in:
ufsbranch: https://github.com/JessicaMeixner-NOAA/ufs-weather-model/tree/bug/debugww3fix
ww3branch: https://github.com/JessicaMeixner-NOAA/WW3/commits/merge-dev-to-ufs-20220324

There is no expectation that any answers will change. I will start running regression tests on all machines I have access to (hera, orion, gaea, WCOSS-DELL, WCOSS-cray) and run the following oRT test:
./opnReqTest -n cpld_control_c96_p8 -c thr,rst,dbg,fhz

I hope to have a PR submitted by the end of the day. If a preview of the PR is wanted before I've finished the tests, I'm happy to do that too.