update rt.conf and compile.sh to provide cmake options and avoid a translation layer #615

aerorahul · 2021-06-02T13:35:28Z

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.
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.

Description

compile.sh reads in the MAKE_OPT which are the deprecated GNU-style options for compilations from rt.conf.
This PR:

replaces GNU-style compile options from rt.conf in cmake style.
updates compile.sh to pass through MAKE_OPT as is to build.sh which calls cmake.
removes confusion as to what is an input to compile.sh and build.sh. E.g. SUITES is (was) input to MAKE_OPT and compile.sh would transform that to -DCCPP_SUITES for use in build.sh. Hereafter, we use -DCCPP_SUITES in rt.conf Similar changes have been made to 32BIT, DEBUG, REPRO, MULTI_GASES, etc.

Issue(s) addressed

Part of #416
More work might be needed based on the comments to this PR.

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

None.

…r-model into develop

into develop

Change-Id: Ie2103eab6d91105f9cd0e9bb069c2d95bef2b6b8

Change-Id: I7d848f4cd91bea74b04d8b43ab1f45a1ace8cda2

Change-Id: I92025fbae25785f84abf7fa1a08bfbca901bffa6

DusanJovic-NOAA · 2021-06-02T15:12:05Z

rt.sh must be updated at two places where MAKE_OPT is used:

if [[ ${MAKE_OPT^^} =~ "REPRO=Y" ]]; then

and:

if [[ ${MAKE_OPT^^} =~ "WW3=Y" ]]; then

aerorahul · 2021-06-02T15:59:41Z

This is going to cause conflicts everytime develop is updated, so I am going to wait till it is this PR's turn to resolve all conflicts.
I will fix the issue in this comment needing an update in rt.sh.

Change-Id: I85510b236d2ef25e09e6f4578ac6196b3a7e2de1

Change-Id: Ie09f7f4cf645bc6661bc49d73fec21e51edfb728

Change-Id: I137a0e798eb2273be61046d081fb379dc0aa0a7a

aerorahul · 2021-06-02T16:35:43Z

rt.sh must be updated at two places where MAKE_OPT is used:

if [[ ${MAKE_OPT^^} =~ "REPRO=Y" ]]; then

and:

if [[ ${MAKE_OPT^^} =~ "WW3=Y" ]]; then

There is no option with WW3=Y|N in MAKE_OPT in any of the conf files.

DusanJovic-NOAA · 2021-06-02T17:12:12Z

rt.sh must be updated at two places where MAKE_OPT is used:
if [[ ${MAKE_OPT^^} =~ "REPRO=Y" ]]; then
and:
if [[ ${MAKE_OPT^^} =~ "WW3=Y" ]]; then

There is no option with WW3=Y|N in MAKE_OPT in any of the conf files.

I see. Then this second if test must be updated to look for APP that includes WW3 (ATMW, S2SW, etc).

Change-Id: I9619f724e12e36607ea84a009134e9997d9bddf0

aerorahul · 2021-06-02T17:16:01Z

rt.sh must be updated at two places where MAKE_OPT is used:
if [[ ${MAKE_OPT^^} =~ "REPRO=Y" ]]; then
and:
if [[ ${MAKE_OPT^^} =~ "WW3=Y" ]]; then

There is no option with WW3=Y|N in MAKE_OPT in any of the conf files.

I see. Then this second if test must be updated to look for APP that includes WW3 (ATMW, S2SW, etc).

done in 0905841. Please take a look again.

Change-Id: Ide8e962ffc1f1304c042bb79907789442180338f

uturuncoglu · 2021-06-03T19:53:07Z

@aerorahul i am not sure but is this PR is for solving the issue that I faced before. The build was trying to add multiple APP in the build.

aerorahul · 2021-06-03T19:55:59Z

@aerorahul i am not sure but is this PR is for solving the issue that I faced before. The build was trying to add multiple APP in the build.

@uturuncoglu Yes I believe so. In this PR, the contents of MAKE_OPT are directly passed to cmake. compile.sh does not translate MAKE_OPT into CMAKE_FLAGS. This will resolve the issue you faced where the build was adding multiple APP's to the build. Please take a look and let me know if the issue persists or creates another one.

uturuncoglu · 2021-06-03T19:57:42Z

@aerorahul Thanks for the clarification. Currently I could not access to Orion and I am trying to setup Cheyenne for testing. I'll let you know when I have a change to test it. Thanks again.

uturuncoglu · 2021-06-09T19:33:25Z

@aerorahul any update about this PR?

uturuncoglu · 2021-06-28T15:50:53Z

@aerorahul @junwang-noaa i am testing this PR along with the #611 and I am getting build error like following when I tried to compile DOCN case,

+ cmake /work/noaa/gmtb/tufuk/UFS_cdeps APP=HAFS_DOCN SUITES=HAFS_v0_gfdlmp_tedmf_nonsst 32BIT=Y -DMPI=ON -DCMAKE_BUILD_TYPE=Release
CMake Error: The source directory "/work/noaa/gmtb/tufuk/UFS_cdeps/tests/build_fv3_001/32BIT=Y" does not exist.
Specify --help for usage, or press the help button on the CMake GUI.

any idea. I'll also review the PR in a more detailed way but first I need to test it under HAFS app with data component configurations.

uturuncoglu · 2021-06-28T15:52:14Z

maybe I need to replace 32BIT=Y with -D32BIT=ON

uturuncoglu · 2021-06-28T15:53:01Z

i also need to add -D before CCPP and APP

uturuncoglu · 2021-06-28T16:00:37Z

@aerorahul okay I pass those errors by adding -D and aand make change in 32BIT option. Now, I am getting following error,

CMake Error: install(EXPORT "CDEPSExports" ...) includes target "dshr" which requires target "streams" that is not in any export set.
CMake Error: install(EXPORT "CDEPSExports" ...) includes target "datm" which requires target "streams" that is not in any export set.
CMake Error: install(EXPORT "CDEPSExports" ...) includes target "dice" which requires target "streams" that is not in any export set.
CMake Error: install(EXPORT "CDEPSExports" ...) includes target "dlnd" which requires target "streams" that is not in any export set.
CMake Error: install(EXPORT "CDEPSExports" ...) includes target "docn" which requires target "streams" that is not in any export set.
CMake Error: install(EXPORT "CDEPSExports" ...) includes target "drof" which requires target "streams" that is not in any export set.
CMake Error: install(EXPORT "CDEPSExports" ...) includes target "dwav" which requires target "streams" that is not in any export set.
CMake Error in CDEPS-interface/CMakeLists.txt:
  export called with target "dshr" which requires target "streams" that is
  not in any export set.


CMake Error in CDEPS-interface/CMakeLists.txt:
  export called with target "datm" which requires target "streams" that is
  not in any export set.

Do I need to use special version of CDEPS? We are using NOAA-EMC CDEPS fork as a base in HAFS app but I have additional modifications for ERA5 data atmosphere in it. So, I am not expecting any issue with the CDEPS version that I am using.

aerorahul · 2021-06-28T16:02:20Z

@uturuncoglu
I apologize, I have not updated my branch with the latest develop. I will do so now.

uturuncoglu · 2021-06-28T16:03:40Z

@aerorahul okay. no problem. let me know when it is ready and I could test it again.

aerorahul · 2021-06-28T16:07:05Z

@uturuncoglu I think I resolved the conflicts. I have not tested them yet.

uturuncoglu · 2021-06-28T16:35:06Z

tests/rt.sh

      RT_SUFFIX="_repro"
      BL_SUFFIX="_repro"
    else
      RT_SUFFIX=""
      BL_SUFFIX=""
    fi

-    if [[ ${MAKE_OPT^^} =~ "WW3=Y" ]]; then
+    if [[ ${MAKE_OPT^^} =~ "-DAPP=ATMW" ]] ||  [[ ${MAKE_OPT^^} =~ "-DAPP=S2SW" ]]; then


I think that this is not flexible that I explain in the following issue #656. I already completed WW3 coupling through the use of CMEPS under HAFS application and created a set of wave coupled configurations. Listing applications names in this level is not flexible and once someone bring another configuration this file needs to be changed.

Please make a suggestion/PR and we can review.
All the applications that are present in the UFS-weather-model are tested and supported. So if someone brings in an "application", that needs to be supported.
The entire business of adding if-then-else needs to be revisited.

aerorahul · 2021-07-07T21:00:32Z

#642 absorbed this PR.
Closing.

junwang-noaa and others added 13 commits October 21, 2019 14:32

point to Jun fv3atm personal fork

0dbb6f6

Merge branch 'develop' of https://github.com/ufs-community/ufs-weathe…

30d4ea2

…r-model into develop

Merge remote-tracking branch 'upstream/develop' into develop

f1ba56c

update to the top of upstream develop branch

927077b

Merge branch 'develop' of https://github.com/aerorahul/ufs-weather-model

734fd49

into develop

Merge remote-tracking branch 'upstream/develop' into develop

678ab1c

Merge remote-tracking branch 'upstream/develop' into develop

5e03cbd

Merge remote-tracking branch 'upstream/develop' into develop

30f8488

Merge remote-tracking branch 'upstream/develop' into develop

2d9e48f

Merge branch 'ufs-community:develop' into develop

4bd1275

build full CDEPS library and link against it instead of data components

302d428

Change-Id: Ie2103eab6d91105f9cd0e9bb069c2d95bef2b6b8

remove erroneous PRIVATE compiler definition

e9210f8

Change-Id: I7d848f4cd91bea74b04d8b43ab1f45a1ace8cda2

cleanup rt.conf and pass directly as CMAKE_FLAGS to compile.sh

28a53f4

Change-Id: I92025fbae25785f84abf7fa1a08bfbca901bffa6

aerorahul requested review from climbfuji, DusanJovic-NOAA and junwang-noaa June 2, 2021 13:35

aerorahul added 3 commits June 2, 2021 12:14

Merge remote-tracking branch 'upstream/develop' into develop

54fc471

Change-Id: I85510b236d2ef25e09e6f4578ac6196b3a7e2de1

Merge branch 'develop' into feature/link_full_cdeps

dfd0348

Change-Id: Ie09f7f4cf645bc6661bc49d73fec21e51edfb728

Merge branch 'feature/link_full_cdeps' into feature/cmake_flags_cleanup

cb8e948

Change-Id: I137a0e798eb2273be61046d081fb379dc0aa0a7a

DusanJovic-NOAA mentioned this pull request Jun 2, 2021

Link full CDEPS library at compile time instead of data components. #611

Closed

12 tasks

fix rt.sh following reviewer comments

0905841

Change-Id: I9619f724e12e36607ea84a009134e9997d9bddf0

junwang-noaa mentioned this pull request Jun 2, 2021

add CDEPS as a library with all data components included #616

Closed

revert CDEPS changes to develop

9c35a0a

Change-Id: Ide8e962ffc1f1304c042bb79907789442180338f

aerorahul mentioned this pull request Jun 15, 2021

Eliminate computational instability in the datm_cdeps_mx025_cfsr test on gaea, orion and wcoss_dell_p3 #642

Merged

14 tasks

junwang-noaa requested a review from uturuncoglu June 17, 2021 14:22

Merge branch 'develop' into feature/cmake_flags_cleanup

4a7fb66

uturuncoglu reviewed Jun 28, 2021

View reviewed changes

aerorahul closed this Jul 7, 2021

aerorahul deleted the feature/cmake_flags_cleanup branch July 7, 2021 21:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update rt.conf and compile.sh to provide cmake options and avoid a translation layer #615

update rt.conf and compile.sh to provide cmake options and avoid a translation layer #615

aerorahul commented Jun 2, 2021 •

edited

Loading

DusanJovic-NOAA commented Jun 2, 2021

aerorahul commented Jun 2, 2021

aerorahul commented Jun 2, 2021

DusanJovic-NOAA commented Jun 2, 2021

aerorahul commented Jun 2, 2021

uturuncoglu commented Jun 3, 2021

aerorahul commented Jun 3, 2021

uturuncoglu commented Jun 3, 2021

uturuncoglu commented Jun 9, 2021

uturuncoglu commented Jun 28, 2021

uturuncoglu commented Jun 28, 2021

uturuncoglu commented Jun 28, 2021

uturuncoglu commented Jun 28, 2021

aerorahul commented Jun 28, 2021

uturuncoglu commented Jun 28, 2021

aerorahul commented Jun 28, 2021

uturuncoglu Jun 28, 2021

aerorahul Jun 28, 2021

aerorahul commented Jul 7, 2021

update rt.conf and compile.sh to provide cmake options and avoid a translation layer #615

update rt.conf and compile.sh to provide cmake options and avoid a translation layer #615

Conversation

aerorahul commented Jun 2, 2021 • edited Loading

PR Checklist

Description

Issue(s) addressed

Testing

Dependencies

DusanJovic-NOAA commented Jun 2, 2021

aerorahul commented Jun 2, 2021

aerorahul commented Jun 2, 2021

DusanJovic-NOAA commented Jun 2, 2021

aerorahul commented Jun 2, 2021

uturuncoglu commented Jun 3, 2021

aerorahul commented Jun 3, 2021

uturuncoglu commented Jun 3, 2021

uturuncoglu commented Jun 9, 2021

uturuncoglu commented Jun 28, 2021

uturuncoglu commented Jun 28, 2021

uturuncoglu commented Jun 28, 2021

uturuncoglu commented Jun 28, 2021

aerorahul commented Jun 28, 2021

uturuncoglu commented Jun 28, 2021

aerorahul commented Jun 28, 2021

uturuncoglu Jun 28, 2021

Choose a reason for hiding this comment

aerorahul Jun 28, 2021

Choose a reason for hiding this comment

aerorahul commented Jul 7, 2021

aerorahul commented Jun 2, 2021 •

edited

Loading