Feature/update mom6 and retain b4b results for 025x025 resolution

[jiandewang]

Description

To follow up GFDL's latest MOM6 main branch updating (20201022), EMC MOM6 repo will make corresponding updating (MOM6 PR No. 40, NOAA-EMC/MOM6#40), this code updating involves several model default value changes which is going to change answers. For the first step of updating the coupled system, we will modify MOM_input for quarter degree setting by switching the changed default values back to their original values to retain original answers. This will require a new baseline as there are 4 variables being removed from one of ocean restart file, and for half and 1x1 degree settings, the answer will change.
There will be no library change in this PR.

Issue(s) addressed

this PR is for issue #283

Testing

Update MOM6 in current ufs-weather-model develop branch with modified MOM_input for quarter degree, run regression tests on orion, hera and dell with intel compiler

Dependencies

waiting on MOM6 PR No. 40, NOAA-EMC/MOM6#40

[aerorahul]

@jiandewang
Does this PR fix the issue with compiling the standalone MOM6 within UFS? That issue was resolved in the GFDL/main branch, so I am hoping this sync should fix it.

This option is triggered by adding the following to CMAKE_FLAGS if you are using just build.sh

CMAKE_FLAGS="-DMOM6SOLO=ON"

There was a bug in the way the wave forcing subroutine was being called with incorrect number of arguments.

[aerorahul]

@jiandewang
It doesn't build MOM6SOLO.

CMAKE_FLAGS="-DMOM6SOLO=ON -DS2S=ON -DDATM=ON" BUILD_VERBOSE=1 BUILD_JOBS=6 BUILD_DIR=build-datmmom6 ./build.sh

[ 99%] Building Fortran object MOM6-interface/CMakeFiles/mom6solo.dir/MOM6/config_src/solo_driver/MOM_driver.F90.o
cd /Users/rmahajan/scratch/UFSwork/weather-develop/build-datmmom6/MOM6-interface && /opt/modules/gnu-9.3.0/mpich/3.3.1/bin/mpifort -DENABLE_QUAD_PRECISION -DGFS_PHYS -DINTERNAL_FILE_NML -Duse_libMPI -Duse_netCDF -I/Users/rmahajan/scratch/UFSwork/weather-develop/build-datmmom6/MOM6-interface/mod -I/Users/rmahajan/scratch/UFSwork/weather-develop/build-datmmom6/MOM6-interface/mod_solo -I/Users/rmahajan/scratch/UFSwork/weather-develop/MOM6-interface/MOM6/config_src/dynamic -I/Users/rmahajan/scratch/UFSwork/weather-develop/MOM6-interface/MOM6/src/framework -I/Users/rmahajan/scratch/UFSwork/weather-develop/FMS/include -I/Users/rmahajan/scratch/UFSwork/weather-develop/FMS/fms -I/Users/rmahajan/scratch/UFSwork/weather-develop/FMS/mpp/include -I/Users/rmahajan/scratch/UFSwork/weather-develop/build-datmmom6/FMS/mod -I/opt/modules/gnu-9.3.0/mpich-3.3.1/netcdf/4.7.4/include -g -fbacktrace -fdefault-real-8 -fdefault-double-8 -Waliasing -fcray-pointer -fconvert=big-endian -ffree-line-length-none -fno-range-check -fbacktrace -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk -mmacosx-version-min=10.15 -Jmod_solo -c /Users/rmahajan/scratch/UFSwork/weather-develop/MOM6-interface/MOM6/config_src/solo_driver/MOM_driver.F90 -o CMakeFiles/mom6solo.dir/MOM6/config_src/solo_driver/MOM_driver.F90.o
/Users/rmahajan/scratch/UFSwork/weather-develop/MOM6-interface/MOM6/config_src/solo_driver/MOM_driver.F90:491:79:

  491 |       call Update_Surface_Waves(grid, GV, US, time, time_step_ocean, waves_csp)
      |                                                                               1
Error: Missing actual argument for argument 'forces' at (1)
make[2]: *** [MOM6-interface/CMakeFiles/mom6solo.dir/MOM6/config_src/solo_driver/MOM_driver.F90.o] Error 1
make[1]: *** [MOM6-interface/CMakeFiles/mom6solo.dir/all] Error 2
make: *** [all] Error 2

[jiandewang]

@aerorahul this is EMC internal issue, what happened is at the time when wave part code was merged in EMC repo, we only tested in coupled driver, didn't test the standalone MOM6, there is an new argument being added in wave forcing subroutine but it was not decleared as optional. Will address this issue in a separate PR.

[aerorahul]

@jiandewang
Can you give me a timeframe for fixing this long standing issue?

[jiandewang]

@aerorahul on my to do list right after this PR

[jiandewang]

MOM6 PR No. 40 has been done NOAA-EMC/MOM6#40
updated MOM6 submodule to pointed to EMC repo

[aerorahul]

Have regression tests been run for this PR with the MOM6 changes?

[jiandewang]

@aerorahul yes

[DeniseWorthen]

For the first of the two-part MOM6 update, I think this is ready for baseline creation and commit. I assume @jiandewang is waiting for his turn in the commit queue.

[jiandewang]

@aerorahul @DeniseWorthen I am think of including Rahul's MOM6 PR NO.43
NOAA-EMC/MOM6#43 to this ufs-weather-model PR, as it will solve solo compiling issue, but won't have any impact in coupled system as the modified file won't be used here.

[aerorahul]

@jiandewang
Thanks. If you do include NOAA-EMC/MOM6#43 in this PR, please add capability in the regression tests to build MOM6SOLO executable for the coupled model (and data atmosphere) compilation.

We don't run the mom6 solo executable, so there is no additional computational resources required.
Building the executable from the mom6 library only takes a few extra seconds.

This will ensure that the mom6solo compilation is not broken again.

If you need assistance, please let me know and I can send you a patch on how to build the mom6solo executable.

[JessicaMeixner-NOAA]

If you're building it, shouldn't you run a solo test too? I get that building it is not an extra computational resource but what is it in support of? By this logic should all the other components (ice, wave, ...) also be built standalone as a test too? If this is just about making sure the EMC fork of MOM6 doesn't break standalone, would adding a standalone test to the MOM6 commit process make more sense than adding it here?

[jiandewang]

@aerorahul this ufs-weather PR is almost ready for merge, so will do add what you mentioned in the future PR

[aerorahul]

@aerorahul this ufs-weather PR is almost ready for merge, so will do add what you mentioned in the future PR

@jiandewang
This is all that is required in the UFS

~/s/U/u/tests ❯❯❯ git diff                                     feature/update-MOM6-retain-b4b-025 ✱
diff --git i/tests/compile.sh w/tests/compile.sh
index 4300685..fc8875d 100755
--- i/tests/compile.sh
+++ w/tests/compile.sh
@@ -145,6 +145,10 @@ if [[ "${MAKE_OPT}" == *"DATM=Y"* ]]; then
     CMAKE_FLAGS="${CMAKE_FLAGS} -DDATM=Y"
 fi

+if [[ "${MAKE_OPT}" == *"S2S=Y"* ]] || [[ ${MAKE_OPT} == *"DATM=Y"* ]]; then
+    CMAKE_FLAGS="${CMAKE_FLAGS} -DMOM6SOLO=ON"
+fi
+
 CMAKE_FLAGS=$(trim "${CMAKE_FLAGS}")

 if [ $clean_before = YES ] ; then

[jiandewang]

@aerorahul will modify the file and let you check

[jiandewang]

@aerorahul shall
-DMOM6SOLO=ON or = Y ?

[aerorahul]

If you're building it, shouldn't you run a solo test too? I get that building it is not an extra computational resource but what is it in support of? By this logic should all the other components (ice, wave, ...) also be built standalone as a test too? If this is just about making sure the EMC fork of MOM6 doesn't break standalone, would adding a standalone test to the MOM6 commit process make more sense than adding it here?

@JessicaMeixner-NOAA
Sounds good. SOLO tests need to be run, but not in UFS. It should be run in the MOM6 repository. When they are run in MOM6, we can disable the build here.
GFDL tests the MOM6 solo (and other coupled applications). We (EMC) do not.
If we (EMC) make changes to MOM6 src/ code and push them upstream to GFDL they will get tested. But, we have not made that effort. Instead, we are carrying around modified files in our local fork/branch, and it is breaking the MOM6 solo application.

[aerorahul]

@aerorahul shall
-DMOM6SOLO=ON or = Y ?

Does not matter. YES|ON|TRUE all are acceptable.

[jiandewang]

@aerorahul is there any change needed in "CMakeLists.txt" ? how do I tested the compiling of MOM6SOLO after modified compile.sh ?

[aerorahul]

@aerorahul is there any change needed in "CMakeLists.txt" ? how do I tested the compiling of MOM6SOLO after modified compile.sh ?

@jiandewang
There are no changes required in the CMakeLists.txt files. If there is an error in building the executable mom6solo, the compile jobs will fail. If they are successful, an executable named mom6solo will be generated in the build directory MOM6-interface/mom6solo.

In the compile log, a line such as this will appear:

[ 96%] Built target mom6solo

I tested with:

+if [[ "${MAKE_OPT}" == *"S2S=Y"* ]] || [[ ${MAKE_OPT} == *"DATM=Y"* ]]; then
+    CMAKE_FLAGS="${CMAKE_FLAGS} -DMOM6SOLO=ON"
+fi

and was able to build the mom6solo executable.

[aerorahul]

@jiandewang
The top-level CMakeLists.txt is still 100755
The tests/compile.sh is now 100644, and should be 100755.

[jiandewang]

Dom and I have already ran baseline and RT, if we switch this parameter we will need to re-run them. For me I am opening to this

…

On Thu, Dec 3, 2020 at 10:44 AM Jessica Meixner ***@***.***> wrote: @jiandewang <https://github.com/jiandewang> @DeniseWorthen <https://github.com/DeniseWorthen> so it even says in the note there "Default is False in order to maintain answers with legacy experiments but should be changed to True for new experiments." Since we're updating settings, should this be one we diverge from GFDL to update to True? Are there others like this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#290 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADMGAS6YSTSYGVOQKYKMFLLSS6W47ANCNFSM4T4JYI4Q> .

-- Jiande Wang IMSG at NOAA/NWS/NCEP/EMC cubic 2088 jiande.wang@noaa.gov phone:3016833725

[JessicaMeixner-NOAA]

Let's just leave the parameters as is and we can revisit them in the future.

[jiandewang]

@aerorahul did chmod for them, thanks

[jiandewang]

@JessicaMeixner-NOAA sounds good, will make a note on this

[DeniseWorthen]

Do we know if GFDL plans a further update where for their own testing/baselines they will finally turn some of these settings to the new defaults?

[jiandewang]

@DeniseWorthen not mentioned so far

[jiandewang]

all RT run are done, log files have been committed