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

use FMS 2020.04.02 CMakeLists.txt #404

Closed
wants to merge 15 commits into from

Conversation

mlee03
Copy link
Contributor

@mlee03 mlee03 commented Feb 1, 2021

Description

This PR updates FMS to version 2020.04.02 and includes three notable changes:

  1. FMS 2020.04.01 will default to use the newer io implementation called fms2_io. To use the old mpp_io that is in 2019.01.03, the namelist variable use_mpp_io=.true. is required for interpolator_nml, xgrid_nml, amip_interp_nml, and diag_manager_nml; and use_mpp_bug=.true. in data_override_nml

  2. An additional option has been implemented in the file section of the diag_table and only affects files using the optional diag_table features. Users can now specify the last field as "begin", "middle", or "end" in order to have more control of the output filename. An example specification is shown below:

    "ocn%4yr%2mo%2dy%2hr", 6,  "hours", 1, "hours", "time", 6, "hours", "1901 1 1 0 0 0", 0, "", "begin"

    Given the above specification, the file corresponding to the first 6 hours of the run will be named
    ocn_2016_10_03_00.nc for "begin"
    ocn_2016_10_03_03.nc for "middle"
    ocn_2016_10_03_06.nc for "end"
    If the last field is not specified , "middle" will be used as default. More details are provided here

  3. FMS CMakeLists.txt is incorporated into the UFS Weather Model CMake build.

Two metadata differences may be observed with nccmp:

  1. NetCDF file format change from NC_FORMAT_NETCDF4_CLASSIC to NC_FORMAT_64BIT. Users can specify netcdf_default_format="classic" in fms2_io_nml to use NC_FORMAT_NETCDF4_CLASSIC.
  2. Checksum differences in FV3 32-bit runs due to a bug fix in mpp_chksum.

Other than these changes, no answer changes are expected.

Issue(s) addressed

Testing

Last round of regression tests are running on Orion with Intel/2018.4. Stay-tuned.

Dependencies

This PR depends on
PR 37 in noaa-psd/stochastic_physics. PR 37 has been merged in and this PR points to the updated stochastic_phyics submodule.
PR 243 in NOAA-EMC/fv3atm

@mlee03 mlee03 marked this pull request as ready for review February 1, 2021 16:34
@mlee03
Copy link
Contributor Author

mlee03 commented Feb 1, 2021

Last round of tests are currently running.

Copy link
Collaborator

@DusanJovic-NOAA DusanJovic-NOAA left a comment

Choose a reason for hiding this comment

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

I do not see FV3 updated submodule, mentioned in PR description.

Copy link
Contributor Author

@mlee03 mlee03 left a comment

Choose a reason for hiding this comment

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

@DusanJovic-NOAA, the changes haven't been merged yet in NOAA-EMC/fv3atm. I will update my branch once the PR has been merged in.

CMakeLists.txt Show resolved Hide resolved
@climbfuji
Copy link
Collaborator

@DusanJovic-NOAA, the changes haven't been merged yet in NOAA-EMC/fv3atm. I will update my branch once the PR has been merged in.

That's unfortunately not how it works. Modify your .gitmodules file to point to your URL and your branch. Update the submodule pointer in the ufs-weather-model PR to your hash (in your fork/branch). We code managers need to be able to check out the code exactly as you use it for testing by doing

git clone -b fms/2020.04.02 --recursive https://github.com/mlee03/ufs-weather-model

i.e. you need to do this for all PRs that have changes in submodules.

@mlee03
Copy link
Contributor Author

mlee03 commented Feb 1, 2021

@climbfuji, thank you for the info, this is my first PR to the repository :)
I have updated the .gitmodule files to point to my forked branch.
Please let me know if I need to make additional changes.

@DusanJovic-NOAA, I have changed the FMS submodule pointer to checkout the 2020.04.02 tag.

@jiandewang
Copy link
Collaborator

@mlee03 the additional option in diag_table is exactly what we need. Thanks.

@DusanJovic-NOAA
Copy link
Collaborator

As pointed out by @aerorahul here https://github.com/NOAA-EMC/fv3atm/pull/243/files#r567918169, none of those updates in various submodules are needed if we add an alias in the top-level CMakeLists.txt. All submodules can just use fms as a dependency.

@aerorahul
Copy link
Contributor

@mlee03
Attached is a diff file you can use to update this PR.
This is the only change required for the UFS to use the FMS tag 2020.04.02.

 /w/n/d/r/w/U/ufsgh-uwm-develop [develop ✚.2 …1] ❯❯❯ cat diff.txt
diff --git i/CMakeLists.txt w/CMakeLists.txt
index e4f62ef..7e9130f 100644
--- i/CMakeLists.txt
+++ w/CMakeLists.txt
@@ -128,43 +128,19 @@ endif()
 ###############################################################################
 ### FMS
 ###############################################################################
-include(fms_files.cmake)
-add_library(fms ${fms_src_files} ${fms_headers})
-# stupid cmake can not figure out dependency of fft.F90 on fft99.F90 because 'use fft99_mod' is inside ifdefs
-set_property(SOURCE FMS/fft/fft.F90 APPEND_STRING PROPERTY COMPILE_FLAGS "-DSGICRAY=0 -DNAGFFT=0")
-
-list(APPEND _fms_defs_public use_libMPI
-                             use_netCDF
-                             GFS_PHYS
-                             INTERNAL_FILE_NML)
+set(GFS_PHYS ON CACHE BOOL "Enable GFS Physics")
+if(NOT 32BIT)
+   set(64BIT ON CACHE BOOL "Enable 64-bit")
+endif()
 if(QUAD_PRECISION)
-  list(APPEND _fms_defs_public ENABLE_QUAD_PRECISION)
+  set(ENABLE_QUAD_PRECISION ON CACHE BOOL "Enable Quad-precision")
 endif()
-target_compile_definitions(fms PUBLIC "${_fms_defs_public}")
-
+add_subdirectory(FMS)
 if(32BIT)
-  list(APPEND _fms_defs_private OVERLOAD_R4
-                                OVERLOAD_R8)
-endif()
-
-target_compile_definitions(fms PRIVATE "${_fms_defs_private}")
-
-target_include_directories(fms PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/FMS/include>
-                                      $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/FMS/fms>
-                                      $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/FMS/fms2_io/include>
-                                      $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/FMS/mod>
-                                      $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/FMS/mpp/include>)
-target_include_directories(fms INTERFACE
-                                      $<INSTALL_INTERFACE:${CMAKE_INSTALL_PREFIX}/include>
-                                      $<INSTALL_INTERFACE:${CMAKE_INSTALL_PREFIX}/mod>)
-target_link_libraries(fms PUBLIC MPI::MPI_Fortran
-                                 NetCDF::NetCDF_Fortran)
-if(OpenMP_Fortran_FOUND)
-  target_link_libraries(fms PRIVATE OpenMP::OpenMP_Fortran)
+  add_library(fms ALIAS fms_r4)
+else()
+  add_library(fms ALIAS fms_r8)
 endif()
-set_target_properties(fms PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/FMS)
-set_target_properties(fms PROPERTIES Fortran_MODULE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/FMS/mod)
-set_target_properties(fms PROPERTIES PUBLIC_HEADER "${fms_headers}" )

 ###############################################################################
 ### stochastic_physics
@@ -331,18 +307,6 @@ target_link_libraries(ufs_model PRIVATE ufs
 ###############################################################################
 ### Install
 ###############################################################################
-install(
-  TARGETS fms
-  EXPORT fms-config
-  LIBRARY DESTINATION lib
-  ARCHIVE DESTINATION lib
-  PUBLIC_HEADER DESTINATION include )
-
-install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/FMS/mod DESTINATION ${CMAKE_INSTALL_PREFIX})
-
-install(EXPORT fms-config
-  DESTINATION lib/cmake
-)
 install(
   TARGETS ufs
   EXPORT ufs-config

@mlee03
Copy link
Contributor Author

mlee03 commented Feb 1, 2021

@aerorahul, the models are compiling with your changes. I will update my branch with your changes.

@mlee03
Copy link
Contributor Author

mlee03 commented Feb 1, 2021

cpld_2threads_prod regression test is not passing

@DeniseWorthen
Copy link
Collaborator

Can you point me to your run directory?

@mlee03
Copy link
Contributor Author

mlee03 commented Feb 2, 2021

@DeniseWorthen, on Orion

cd /work/noaa/gfdlscr/mlee/FINAL_UFS_PR_2020.04.02/ufs-weather-model/tests/mlee/FV3_RT/rt_263461/cpld_2threads_prod

@DeniseWorthen
Copy link
Collaborator

Thanks. I'll take a look at the 2thread case. I also notice you had two other failures but perhaps those were expected?

Test fv3_ccpp_gfdlmprad_atmwav 018 failed in check_result failed
Test fv3_ccpp_gfdlmprad_atmwav 018 failed in run_test failed
Test cpld_2threads 073 failed in check_result failed
Test cpld_2threads 073 failed in run_test failed
Test fv3_ccpp_wrtGauss_nemsio_c768 019 failed failed
Test fv3_ccpp_wrtGauss_nemsio_c768 019 failed in run_test failed

@mlee03
Copy link
Contributor Author

mlee03 commented Feb 2, 2021

@DeniseWorthen, I think 18 and 19 failed due to Orion issues. Test 18 ran out of runtime,

"slurmstepd: error: *** JOB 1165551 ON Orion-10-01 CANCELLED AT 2021-02-01T09:14:00 DUE TO TIME LIMIT ***"

and 19 gave

"642: Fatal error in PMPI_Wait: Internal MPI error!, error stack:
642: [642:Orion-06-42] unexpected DAPL connection event 0x4008 from 664"

@DeniseWorthen
Copy link
Collaborator

Thanks. I couldn't look at the other two tests because of permission issues.

@DeniseWorthen
Copy link
Collaborator

The problem I'm finding is that MOM6 is not exporting identical values back to the coupler at the third pass through the run sequence. I need to do some more testing and will keep you posted.

@mlee03
Copy link
Contributor Author

mlee03 commented Feb 2, 2021

Perhaps I missed something when editing the CMakeLists.txt file in MOM6-interface, I'll look into it. All tests passed before the cmake build was edited to bring in the FMS CMakeLists.txt.

@mlee03
Copy link
Contributor Author

mlee03 commented Feb 2, 2021

@DeniseWorthen @jiandewang @junwang-noaa, I'm getting the feeling cpld_2threads regression test fail is not due to cmake build changes. FMS 2020.04.01 passed when tested with commit 742bf4e from Dec 31. That UFS weather model code did not include changes from commit 6a4c307 from Jan 27 which includes "MOM6 bugfixes, GFDL update,... " I will retest FMS 2020.04.01 to see if 2020.04.01 fails with the latest on the develop branch.

@DeniseWorthen
Copy link
Collaborator

I've been testing cpld_2threads in the current develop vs this branch. I put everything in a sequential run sequence and am running at a single time interval for all the component timesteps and coupling. This is all just to help isolate the coupler history differences. What I see is that the fields imported from the ocean are all different at the second coupling timestep. The difference fields seem to have some odd linear features near the equator? For example, this is the difference in ocean surface temperature imported to CMEPS:

Screen Shot 2021-02-02 at 12 41 14 PM

@DeniseWorthen
Copy link
Collaborator

I will try w/o the halo update in the nuopc_driver.

@DeniseWorthen
Copy link
Collaborator

I commented out both the halo fix and the fprec fix and got the same results. The GFDL updates are our own changes coming back to us after they were merged into the main branch at GFDL.

If I revert to your 3a39352 commit, will that correctly build w/o the change that pointed to an fms library?

@DeniseWorthen
Copy link
Collaborator

DeniseWorthen commented Feb 2, 2021

I'm stumped. I built and ran your 3a39352 commit and I also built and ran both this branch and the develop in debug mode. In all cases I'm getting the same result for the cpl_2threads test that the ocean fields exported by MOM6 to the mediator are different on the third pass through the run sequence.

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Feb 3, 2021 via email

@DeniseWorthen
Copy link
Collaborator

DeniseWorthen commented Feb 3, 2021

No, I do not think it is related. This is the non-benchmark config where LI_2016 has always been false. Also, that issue related to restart reproducibility and had a very specific signal of the grid decomposition appearing in the MOM6 fields after restart. In this case, the fields are simply different w/ no coherent pattern. When I tested the halo-fix for the commit to ufs-weather, I found that it reproduced current baselines except for the benchmark cases. And, I tested the halo fix here by reverting that change and obtained the same results.

@mlee03
Copy link
Contributor Author

mlee03 commented Feb 3, 2021

@DeniseWorthen, sorry for the late reply. Commit 3a39352 however will still use fms2_io and use FMS CMakeLists.txt.

  1. I ran FMS 2020.04.01 (which doesn't use FMS CMakeLists.txt for compiling) with UFS Weather Model commit ea8a7aa and the cpld_2threads test passes. The log files are in
cd /work/noaa/gfdlscr/mlee/PRfor2020.04.01/test_feb02_2021_asis/tests/log_orion.intel
  1. I am trying to run FMS 2020.04.01 with the latest in the UFS Weather Model develop branch and getting the error,
Error in opening the compiled module file.  Check INCLUDE paths.   [ENS_CPLCOMP_ESMFMOD]
      USE ENS_CplComp_ESMFMod,ONLY: ENS_CplCompSetServices

The log files are in

 cd /work/noaa/gfdlscr/mlee/PRfor2020.04.01/test_feb02_2021_with_latestUFS/tests/log_orion.intel 

Copy link
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

Looks good

@DeniseWorthen
Copy link
Collaborator

The last commit to ufs-weather removed some legacy code from NEMS. I think you need the change in the top level CMakeLists.txt: here

@mlee03
Copy link
Contributor Author

mlee03 commented Feb 3, 2021

cpld_2threads test pass with 2020.04.01 and the latest on the develop branch commit e3983a0

 cd /work/noaa/gfdlscr/mlee/PRfor2020.04.01/test_feb02_2021_with_latestUFS/tests 

@DeniseWorthen
Copy link
Collaborator

And the difference between the 2020.04.01 and .02 is solely the cmake build?

@mlee03
Copy link
Contributor Author

mlee03 commented Feb 3, 2021

@DeniseWorthen, Yes, cmake should be the only difference

Copy link
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

This may cover some part of your cpld threads issue

CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the changes in CMakeLists.txt

@DeniseWorthen
Copy link
Collaborator

I'll also test it. I assume I'll see the same behaviour as before but best to check.

@DeniseWorthen
Copy link
Collaborator

I get the same failure in with the latest commit too.

@mlee03
Copy link
Contributor Author

mlee03 commented Feb 25, 2021

This PR is being closed and a new PR for FMS 2020.04.03 will be made. FMS 2020.04.03 will have CMake support that is compatible with the UFS Weather Model CMake build.

@mlee03 mlee03 closed this Feb 25, 2021
epic-cicd-jenkins pushed a commit that referenced this pull request Apr 17, 2023
* Remove ENV_INIT_SCRIPT/init_env and source /etc/profile in lmod-setup.

* Move TOPO_DIR and SFC_CLIMO_INPUT_DIR to appropriate sections.

* Bug fix for orion and wcoss2 machine files.

* Remove ENV_INIT from wcoss2 machine file.

* Modify setting of +u/-u.

* Do the same for set +e/-e

* Minor modification.

* Hack for gaea python3 loading.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update FMS to 2020.04.01
7 participants