Update CDEPS submodule and CDEPS tests #599

binli2337 · 2021-05-25T19:11:21Z

PR Checklist

Ths PR is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR. Please consult the ufs-weather-model wiki if you are unsure how to do this.
This PR has been tested using a branch which is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR
An Issue describing the work contained in this PR has been created either in the subcomponent(s) or in the ufs-weather-model. The Issue should be created in the repository that is most relevant to the changes in contained in the PR. The Issue and the dependent sub-component PR
are specified below.
If new or updated input data is required by this PR, it is clearly stated in the text of the PR.

Instructions: All subsequent sections of text should be filled in as appropriate.

The information provided below allows the code managers to understand the changes relevant to this PR, whether those changes are in the ufs-weather-model repository or in a subcomponent repository. Ufs-weather-model code managers will use the information provided to add any applicable labels, assign reviewers and place it in the Commit Queue. Once the PR is in the Commit Queue, it is the PR owner's responsiblity to keep the PR up-to-date with the develop branch of ufs-weather-model.

Description

Change application name "DATM" to "NG-GODAS".
Change application name "DATM-NEMS" to "NG-GODAS-NEMSDATM".
Use a mesh file in MOM6 cap instead of a grid file.
Point submodule CDEPS to NOAA-EMC/CDEPS develop branch.

Issue(s) addressed

Link the issues to be closed with this PR, whether in this repository, or in another repository.
(Remember, issues must always be created before starting work on a PR branch!)

Testing

How were these changes tested? What compilers / HPCs was it tested with? Are the changes covered by regression tests? (If not, why? Do new tests need to be added?) Have regression tests and unit tests (utests) been run? On which platforms and with which compilers? (Note that unit tests can only be run on tier-1 platforms)

Dependencies

If testing this branch requires non-default branches in other repositories, list them. Those branches should have matching names (ideally).

Do PRs in upstream repositories need to be merged first?
If so add the "waiting for other repos" label and list the upstream PRs

waiting on noaa-emc/nems/pull/<pr_number>
waiting on noaa-emc/fv3atm/pull/<pr_number>

* Change application name "DATM" to "NG-GODAS" * Change application name "DATM-NEMS" to "NG-GODAS-NEMSDATM" * Use a mesh file in MOM6 cap instead of a grid file

github-actions · 2021-05-25T19:31:20Z

@binli2337 please bring these up to date with respective authoritative repositories

cdeps NOT up to date

DeniseWorthen · 2021-05-26T14:06:31Z

@binli2337 Please add a fix to the rt_35d.conf. The first compile line is commented out but please add it there too.

Add a comma between the two suite file names:

FV3_GFS_v16_coupled, FV3_GFS_v16_coupled_noahmp

github-actions · 2021-05-26T14:14:39Z

@binli2337 please bring these up to date with respective authoritative repositories

cdeps NOT up to date

tests/rt_35d.conf

DeniseWorthen · 2021-05-26T14:18:37Z

@binli2337 New baseline (develop-20210526) is ready on Cheyenne.

add RegressionTests_hera.gnu.log add RegressionTests_wcoss_cray.log

github-actions · 2021-05-26T15:32:49Z

@binli2337 please bring these up to date with respective authoritative repositories

cdeps NOT up to date

on-behalf-of @ufs-community

github-actions · 2021-05-26T16:01:58Z

@binli2337 please bring these up to date with respective authoritative repositories

cdeps NOT up to date

MinsukJi-NOAA · 2021-05-26T16:08:35Z

The github-actions repo-check error seen above is due to the new CDEPS submodule pointing to develop. This issue need to be fixed after this PR commits.

on-behalf-of @ufs-community

climbfuji · 2021-05-26T16:11:03Z

tests/compile.sh

-
-if [[ "${MAKE_OPT}" == *"APP=DATM_NEMS"* ]]; then
-    CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=DATM_NEMS"
+#if [[ "${MAKE_OPT}" == *"APP=DATM"* ]]; then


Can we remove the commented lines 152-154 and 159-161 ?

DeniseWorthen · 2021-05-26T16:57:43Z

CMakeLists.txt

@@ -16,7 +16,7 @@ list(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/CMakeModules/Modules)
 ###############################################################################

 # Valid applications and choices
-list(APPEND VALID_APPS ATM ATMW S2S S2SW DATM DATM_NEMS)
+list(APPEND VALID_APPS ATM ATMW S2S S2SW NG-GODAS NG-GODAS-NEMSDATM)


No changes have been made in the NEMS driver to switch these names---it will still use "FRONT_NEMS_DATM" and "FRONT_CDEPS_DATM" which then asks for "model" names nems_datm or datm. It seems we're introducing a lot of inconsistency.

@DeniseWorthen You are correct.

@DeniseWorthen My understanding is that this is actually to avoid confusion. in NEMS driver we specify the subcomponent models, not the applications names. Using DATM or NEMS_DATM for both subcomponent and application names could be confusing. Here the NG_GODAS is application name, this application will still define the subcomponents DATM (cdeps datm), MOM6 and CICE6 etc. While before we use DATM as application name and use DATM also as DATM subcomponent along with other subcomponents.

Are both NG-GODAS and NG-GODAS-NEMSDATM needed? What's the difference?

NG-GODAS refers to "mom6_cice6_cdeps_datm" application.
NG-GODAS-NEMSDATM refers to "mom_6_cice6_nems_datm" application.

OK. But do we need both? Didn't we switch to CDEPS, why do we need nems? Are the results different when CDEPS is used compared to NEMS? Are different atm data used?

In the future, NEMS_DATM will be removed from ufs-weather-model. See a related issue at #565.

@junwang-noaa I see, thanks. I do see how DATM is being used two ways (app and component) so this is probably better.

on-behalf-of @ufs-community

github-actions · 2021-05-26T18:47:19Z

@binli2337 please bring these up to date with respective authoritative repositories

cdeps NOT up to date

MinsukJi-NOAA · 2021-05-26T18:51:24Z

Since actions thinks the cdeps is not up to date, tests do not run. I suggest we skip it for this PR.

binli2337 · 2021-05-26T18:53:40Z

@MinsukJi-NOAA OK.

github-actions · 2021-05-26T20:15:22Z

@binli2337 please bring these up to date with respective authoritative repositories

cdeps NOT up to date

uturuncoglu · 2021-05-26T20:59:59Z

@binli2337 There could be an issue in this naming convention (NG-GODAS NG-GODAS-NEMSDATM). I found that compile.sh string check for apps could have issue and might add both of app names to CMAKE_FLAGS when you set APP=NG-GODAS-NEMSDATM. You could easily test it by building APP=NG-GODAS-NEMSDATM and check the CMAKE_FLAGS in the standard out. The details of the issue is in here (see end of the conversation), #416

uturuncoglu · 2021-05-26T21:00:48Z

@binli2337 maybe it has issue if you have _ in the app name and having - will be fine. At this point, I am not sure.

uturuncoglu · 2021-05-27T16:27:07Z

@binli2337 i checked it with - also and the result is same. The build includes multiple apps to CMAKE_FLAGS.

uturuncoglu · 2021-05-27T17:07:31Z

Here is the proposed solution for compile.sh. This isolates APP part and works for following cases,

./compile.sh "$target" "CCPP=Y APP=HAFS_DOCN STATIC=Y SUITES=HAFS_v0_gfdlmp_tedmf_nonsst,HAFS_v0_gfdlmp_tedmf,HAFS_v0_gfdlmp_nocpnsst,HAFS_v0_gfdlmp_nonsst,HAFS_v0_gfdlmp_nocp,HAFS_v0_gfdlmp,HAFS_v0_hwrf_thompson,HAFS_v0_hwrf 32BIT=Y" 32bit YES NO
./compile.sh "$target" "APP=HAFS_DOCN CCPP=Y STATIC=Y SUITES=HAFS_v0_gfdlmp_tedmf_nonsst,HAFS_v0_gfdlmp_tedmf,HAFS_v0_gfdlmp_nocpnsst,HAFS_v0_gfdlmp_nonsst,HAFS_v0_gfdlmp_nocp,HAFS_v0_gfdlmp,HAFS_v0_hwrf_thompson,HAFS_v0_hwrf 32BIT=Y" 32bit YES NO

So, order of APP definition does not affect the results and it match always the correct one. I'll push this changes into my HAFS CDEPS branch that could god eventually to upstream repository.

 # Valid applications
-if [[ "${MAKE_OPT}" == *"APP=ATM"* ]]; then
+APP_OPT=$( echo $MAKE_OPT | tr " " "\n" | sed -n '/APP/p;/APP/q' )
+echo "APP_OPT = ${APP_OPT}"
+
+if [[ "${APP_OPT}" == "APP=ATM" ]]; then
     echo "MAKE_OPT = ${MAKE_OPT}"
     CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=ATM"
 fi
 
-if [[ "${MAKE_OPT}" == *"APP=ATMW"* ]]; then
+if [[ "${APP_OPT}" == "APP=ATMW" ]]; then
     CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=ATMW"
 fi
 
-if [[ "${MAKE_OPT}" == *"APP=S2S"* ]]; then
+if [[ "${APP_OPT}" == "APP=S2S" ]]; then
     CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=S2S -DMOM6SOLO=ON"
 fi
 
-if [[ "${MAKE_OPT}" == *"APP=S2SW"* ]]; then
+if [[ "${APP_OPT}" == "APP=S2SW" ]]; then
     CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=S2SW -DMOM6SOLO=ON"
 fi
 
-if [[ "${MAKE_OPT}" == *"APP=HAFS"* ]]; then
+if [[ "${APP_OPT}" == "APP=HAFS" ]]; then
     CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=HAFS"
 fi
 
-if [[ "${MAKE_OPT}" == *"APP=HAFSW"* ]]; then
+if [[ "${APP_OPT}" == "APP=HAFSW" ]]; then
     CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=HAFSW"
 fi
 
-if [[ "${MAKE_OPT}" == *"APP=DATM"* ]]; then
+if [[ "${APP_OPT}" == "APP=HAFS_DATM" ]]; then
+    CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=HAFS_DATM"
+fi
+
+if [[ "${APP_OPT}" == "APP=HAFS_DOCN" ]]; then
+    CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=HAFS_DOCN"
+fi
+

climbfuji · 2021-05-27T17:19:52Z

Here is the proposed solution for compile.sh. This isolates APP part and works for following cases,

./compile.sh "$target" "CCPP=Y APP=HAFS_DOCN STATIC=Y SUITES=HAFS_v0_gfdlmp_tedmf_nonsst,HAFS_v0_gfdlmp_tedmf,HAFS_v0_gfdlmp_nocpnsst,HAFS_v0_gfdlmp_nonsst,HAFS_v0_gfdlmp_nocp,HAFS_v0_gfdlmp,HAFS_v0_hwrf_thompson,HAFS_v0_hwrf 32BIT=Y" 32bit YES NO

./compile.sh "$target" "APP=HAFS_DOCN CCPP=Y STATIC=Y SUITES=HAFS_v0_gfdlmp_tedmf_nonsst,HAFS_v0_gfdlmp_tedmf,HAFS_v0_gfdlmp_nocpnsst,HAFS_v0_gfdlmp_nonsst,HAFS_v0_gfdlmp_nocp,HAFS_v0_gfdlmp,HAFS_v0_hwrf_thompson,HAFS_v0_hwrf 32BIT=Y" 32bit YES NO

So, order of APP definition does not affect the results and it match always the correct one. I'll push this changes into my HAFS CDEPS branch that could god eventually to upstream repository.
 # Valid applications
-if [[ "${MAKE_OPT}" == *"APP=ATM"* ]]; then
+APP_OPT=$( echo $MAKE_OPT | tr " " "\n" | sed -n '/APP/p;/APP/q' )
+echo "APP_OPT = ${APP_OPT}"
+
+if [[ "${APP_OPT}" == "APP=ATM" ]]; then
     echo "MAKE_OPT = ${MAKE_OPT}"
     CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=ATM"
 fi
 
-if [[ "${MAKE_OPT}" == *"APP=ATMW"* ]]; then
+if [[ "${APP_OPT}" == "APP=ATMW" ]]; then
     CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=ATMW"
 fi
 
-if [[ "${MAKE_OPT}" == *"APP=S2S"* ]]; then
+if [[ "${APP_OPT}" == "APP=S2S" ]]; then
     CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=S2S -DMOM6SOLO=ON"
 fi
 
-if [[ "${MAKE_OPT}" == *"APP=S2SW"* ]]; then
+if [[ "${APP_OPT}" == "APP=S2SW" ]]; then
     CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=S2SW -DMOM6SOLO=ON"
 fi
 
-if [[ "${MAKE_OPT}" == *"APP=HAFS"* ]]; then
+if [[ "${APP_OPT}" == "APP=HAFS" ]]; then
     CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=HAFS"
 fi
 
-if [[ "${MAKE_OPT}" == *"APP=HAFSW"* ]]; then
+if [[ "${APP_OPT}" == "APP=HAFSW" ]]; then
     CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=HAFSW"
 fi
 
-if [[ "${MAKE_OPT}" == *"APP=DATM"* ]]; then
+if [[ "${APP_OPT}" == "APP=HAFS_DATM" ]]; then
+    CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=HAFS_DATM"
+fi
+
+if [[ "${APP_OPT}" == "APP=HAFS_DOCN" ]]; then
+    CMAKE_FLAGS="${CMAKE_FLAGS} -DAPP=HAFS_DOCN"
+fi
+

CCPP=Y and STATIC=Y are no longer required, they are the only=default options

uturuncoglu · 2021-05-27T17:23:32Z

@climbfuji The build add more APP names to CMAKE_FLAGS even if you only provide APP option without CCPP and STATIC ones. So, even for it, we still need fix.

climbfuji · 2021-05-27T17:27:06Z

@climbfuji The build add more APP names to CMAKE_FLAGS even if you only provide APP option without CCPP and STATIC ones. So, even for it, we still need fix.

I was only commenting on CCPP=Y and STATIC=Y no longer being needed, nothing else.

…-community#599) * Improve radiative fluxes and cloud cover in ccpp/physics for HR1

* Point CDEPS to develop branch of NOAA-EMC/CDEPS

9def9dc

* Change application name "DATM" to "NG-GODAS" * Change application name "DATM-NEMS" to "NG-GODAS-NEMSDATM" * Use a mesh file in MOM6 cap instead of a grid file

binli2337 requested a review from junwang-noaa May 25, 2021 19:13

junwang-noaa approved these changes May 25, 2021

View reviewed changes

binli2337 self-assigned this May 25, 2021

binli2337 added the Baseline Updates Current baselines will be updated. label May 25, 2021

binli2337 added 2 commits May 26, 2021 10:21

update rt.sh and rt.conf

0c00e4f

add RegressionTests_wcoss_dell_p3.log

03f8a42

update rt_35d.conf

7584d31

binli2337 added the gaea-intel-RT label May 26, 2021

BrianCurtis-NOAA removed the gaea-intel-RT label May 26, 2021

DeniseWorthen reviewed May 26, 2021

View reviewed changes

tests/rt_35d.conf Show resolved Hide resolved

binli2337 added 3 commits May 26, 2021 10:20

update rt_35d.conf

38d0946

add RegressionTests_hera.intel.log

07700f6

update rt_gnu.conf

4fe09cd

add RegressionTests_hera.gnu.log add RegressionTests_wcoss_cray.log

binli2337 added the cheyenne-gnu-RT label May 26, 2021

RT JOBS PASSED: gaea.intel. Log file uploaded.

6d6e9a9

on-behalf-of @ufs-community

BrianCurtis-NOAA removed the cheyenne-gnu-RT label May 26, 2021

binli2337 added the cheyenne-intel-RT label May 26, 2021

RT JOBS PASSED: cheyenne.gnu. Log file uploaded.

61786e5

on-behalf-of @ufs-community

climbfuji reviewed May 26, 2021

View reviewed changes

BrianCurtis-NOAA removed the cheyenne-intel-RT label May 26, 2021

update compile.sh

9447697

climbfuji approved these changes May 26, 2021

View reviewed changes

DeniseWorthen reviewed May 26, 2021

View reviewed changes

BrianCurtis-NOAA and others added 2 commits May 26, 2021 11:21

RT JOBS PASSED: cheyenne.intel. Log file uploaded.

430895a

on-behalf-of @ufs-community

add RegressionTests_jet.intel.log

39d6b36

binli2337 added the run-ci label May 26, 2021

github-actions bot removed the run-ci label May 26, 2021

add RegressionTests_orion.intel.log

ccaa2d9

DeniseWorthen approved these changes May 26, 2021

View reviewed changes

binli2337 added the Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked. label May 26, 2021

binli2337 merged commit d0f0324 into ufs-community:develop May 26, 2021

climbfuji mentioned this pull request May 31, 2021

Update gsl/develop from develop 2021/05/31 NOAA-GSL/ufs-weather-model#83

Merged

binli2337 deleted the feature/cdeps_tests branch June 11, 2021 00:15

pjpegion pushed a commit to NOAA-PSL/ufs-weather-model that referenced this pull request Apr 4, 2023

Improve radiative fluxes and cloud cover in ccpp/physics for HR1 (ufs…

3c8c247

…-community#599) * Improve radiative fluxes and cloud cover in ccpp/physics for HR1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update CDEPS submodule and CDEPS tests #599

Update CDEPS submodule and CDEPS tests #599

binli2337 commented May 25, 2021 •

edited

Loading

github-actions bot commented May 25, 2021

DeniseWorthen commented May 26, 2021

github-actions bot commented May 26, 2021

DeniseWorthen commented May 26, 2021

github-actions bot commented May 26, 2021

github-actions bot commented May 26, 2021

MinsukJi-NOAA commented May 26, 2021

climbfuji May 26, 2021

DeniseWorthen May 26, 2021

binli2337 May 26, 2021

junwang-noaa May 26, 2021

DusanJovic-NOAA May 26, 2021

binli2337 May 26, 2021

DusanJovic-NOAA May 26, 2021

binli2337 May 26, 2021

DeniseWorthen May 26, 2021

github-actions bot commented May 26, 2021

MinsukJi-NOAA commented May 26, 2021

binli2337 commented May 26, 2021

github-actions bot commented May 26, 2021

uturuncoglu commented May 26, 2021

uturuncoglu commented May 26, 2021

uturuncoglu commented May 27, 2021 •

edited

Loading

uturuncoglu commented May 27, 2021

climbfuji commented May 27, 2021

uturuncoglu commented May 27, 2021

climbfuji commented May 27, 2021

Update CDEPS submodule and CDEPS tests #599

Update CDEPS submodule and CDEPS tests #599

Conversation

binli2337 commented May 25, 2021 • edited Loading

PR Checklist

Instructions: All subsequent sections of text should be filled in as appropriate.

Description

Issue(s) addressed

Testing

Dependencies

github-actions bot commented May 25, 2021

DeniseWorthen commented May 26, 2021

github-actions bot commented May 26, 2021

DeniseWorthen commented May 26, 2021

github-actions bot commented May 26, 2021

github-actions bot commented May 26, 2021

MinsukJi-NOAA commented May 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 26, 2021

MinsukJi-NOAA commented May 26, 2021

binli2337 commented May 26, 2021

github-actions bot commented May 26, 2021

uturuncoglu commented May 26, 2021

uturuncoglu commented May 26, 2021

uturuncoglu commented May 27, 2021 • edited Loading

uturuncoglu commented May 27, 2021

climbfuji commented May 27, 2021

uturuncoglu commented May 27, 2021

climbfuji commented May 27, 2021

binli2337 commented May 25, 2021 •

edited

Loading

uturuncoglu commented May 27, 2021 •

edited

Loading