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

Add CMake build for public release #27

Merged
merged 13 commits into from
Oct 9, 2020
Merged

Add CMake build for public release #27

merged 13 commits into from
Oct 9, 2020

Conversation

kgerheiser
Copy link
Contributor

@kgerheiser kgerheiser commented Sep 30, 2020

This is a draft PR for the CMake build for the public release. It builds, but I think some things (libraries, executables) need to be moved around to have the expected layout.

To build, have the necessary modules loaded and run:

mkdir build
cd build
cmake .. -DCMAKE_INSTALL_PREFIX=<prefix>
make

DESCRIPTION OF CHANGES:

Adds a CMake build usinig ExternalProject_Add

TESTS CONDUCTED:

  • Cheyenne: Build and end-to-end test ("DOT_OR_USCORE" test case) was successful on Cheyenne with intel, both for the cmake build and the old build script (that will soon be deprecated). Path to tests: /glade/scratch/kavulich/UFS_CAM/testing/SRW_PR_27/expt_dirs/
  • Hera: Build and end-to-end tests successful (aside from expected failures). Path to tests: /scratch2/BMC/det/kavulich/workdir/SRW_PR_27/expt_dirs
  • Jet: Build test was successful. Machine is having disk errors that are preventing me from running the end-to-end tests.

@kgerheiser
Copy link
Contributor Author

I also see that the regular build reads from a file to determine CCPP_SUITES. I hardcoded it into CMakeLists.txt. That could be added pretty easily.

@mkavulich
Copy link
Collaborator

I have successfully built the full SRW App on Cheyenne, Hera, and Jet for intel. I have run the SRW App end-to-end on a test case on Cheyenne. Currently debugging some library issues on Hera end-to-end tests; once that is complete I think this PR is ready for merge.

Note that this PR must be merged at the same time as regional_workflow PR#304.

@jwolff-ncar
Copy link
Collaborator

jwolff-ncar commented Oct 7, 2020 via email

@mkavulich
Copy link
Collaborator

@jwolff-ncar The plan is to have individual README files in the docs/ directory for each individual build platform, in the same way NCEPLIBS-external does. Right now we have:

  • README_cheyenne_intel.txt
  • README_hera_intel.txt
  • README_jet_intel.txt

@jwolff-ncar
Copy link
Collaborator

jwolff-ncar commented Oct 7, 2020 via email

@mkavulich mkavulich marked this pull request as ready for review October 8, 2020 08:02
Copy link

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

Copy link
Collaborator

@mkavulich mkavulich left a comment

Choose a reason for hiding this comment

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

Given the Jet file system problems I will not delay this PR any further, and call the Cheyenne and Hera testing sufficient. I'll merge this and https://github.com/NOAA-EMC/regional_workflow/pull/304 shortly, and then cherry-pick the appropriate commits to the appropriate release branches.

Externals.cfg Outdated
branch = feature/regional_release_STRING
#hash = e5419633
repo_url = https://github.com/JeffBeck-NOAA/UFS_UTILS
branch = feature/regional_release
Copy link
Collaborator

Choose a reason for hiding this comment

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

So we're pointing to the HEAD of feature/regional_release, not a tag?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree that pointing to the head of this branch is dangerous given the active nature of that PR; I will go ahead and replace that with a hash.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought we were supposed to be pointing to Jeff's tag: v2.0alphav01

Copy link
Collaborator

Choose a reason for hiding this comment

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

I used this in my test on Monday
repo_url = https://github.com/JeffBeck-NOAA/UFS_UTILS
tag = v2.0alpha01

Copy link
Collaborator

Choose a reason for hiding this comment

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

I did not remember this Jamie, thanks for pointing it out. v2.0alpha01 is the same hash as the head of feature/regional_release so the results will be the same. I'll change the Externals.cfg to use that tag

@mkavulich mkavulich linked an issue Oct 8, 2020 that may be closed by this pull request
4 tasks
@gsketefian
Copy link
Collaborator

The backwards-compatibility changes look ok to me, but really best to do a build the old way to make sure it's ok. If you've done that, then that's fine!

@gsketefian
Copy link
Collaborator

@mkavulich I think this is related, but let me know if I'm just confused...
This morning, I got the latest version of this repo and tried to build HEAD of the develop branch of the ufs-weather-model with it. It encountered an error on the last line of build_forecast.sh:

cp ${model_top_dir}/ufs_weather_model ${model_top_dir}/tests/fv3.exe

Here, the executable is assumed to be "ufs_weather_model", but I had to change it to "ufs_model" for things to work. Also, in this and the sister PR into regional_workflow, the executable is assumed to be NEMS.x (I think), not ufs_weather_model or ufs_model. Any idea what's going on?

@mkavulich
Copy link
Collaborator

Yes @gsketefian I have run back-compatability tests, but only on Cheyenne (where I had committed locally) so I didn't catch that I had forgotten to push those changes here. I am running an extra test on Hera right now using the old build method just to make sure everything is in order.

I am confused about the error you are seeing...in the old way of building, build_forecast.sh calls the build.sh script in the ufs-weather-model, which at the end builds the NEMS.exe executable in ufs-weather-model/build and then copies it to ufs_weather_model/ufs_weather_model...I'm not sure why yours would be named ufs_model instead. I'm using the HEAD of ufs-weather-model:release/public-v2, which is what ufs-srweather-app:master should be pointing to

The new CMake build bypasses that build script entirely and just creates NEMS.exe, which is then copied to bin/fv3_gfs.x by the generate workflow script.

@JulieSchramm
Copy link

JulieSchramm commented Oct 8, 2020 via email

@gsketefian
Copy link
Collaborator

Yes @gsketefian I have run back-compatability tests, but only on Cheyenne (where I had committed locally) so I didn't catch that I had forgotten to push those changes here. I am running an extra test on Hera right now using the old build method just to make sure everything is in order.

I am confused about the error you are seeing...in the old way of building, build_forecast.sh calls the build.sh script in the ufs-weather-model, which at the end builds the NEMS.exe executable in ufs-weather-model/build and then copies it to ufs_weather_model/ufs_weather_model...I'm not sure why yours would be named ufs_model instead. I'm using the HEAD of ufs-weather-model:release/public-v2, which is what ufs-srweather-app:master should be pointing to

The new CMake build bypasses that build script entirely and just creates NEMS.exe, which is then copied to bin/fv3_gfs.x by the generate workflow script.

@mkavulich It must be because I was getting the HEAD of the develop branch of ufs-community/ufs-weather-model. It was for the DTC-PAS project. There, the executable ufs_weather_model is renamed to ufs_model, I suppose because it can include not only a weather model but also an ocean model, and in S2S applications, it's more like a climate model than a weather model. I guess you'll encounter this change when you start using the develop branch again.

@mkavulich mkavulich merged commit eee801a into ufs-community:master Oct 9, 2020
mkavulich added a commit to ufs-community/regional_workflow that referenced this pull request Oct 9, 2020
…ach repository (#304)

## DESCRIPTION OF CHANGES: 
The new top-level cmake build for the SRW App ([SRW App PR#27](ufs-community/ufs-srweather-app#27)) results in some executables having different names. This PR makes modifications that
 1. Allow the workflow to run successfully with the new cmake build and its different executable names, and
 2. Allow back-compatibility with the old build system to allow for a gradual transition to new build system

This PR also explicitly disallows running the workflow without CCPP, which we decided against supporting several months ago. I don't think the capability even works so this shouldn't effect anyone at this time.

## TESTS CONDUCTED: 
 - **Cheyenne**: Build and end-to-end test ("DOT_OR_USCORE" test case) was successful on Cheyenne with intel, both for the cmake build and the old build script (that will soon be deprecated). 
 - **Hera**: Build and end-to-end tests successful (aside from expected failures). Also built with old build script successfully.
 - **Jet**: Build test was successful. 

## ISSUE: 
It was not the primary aim of this PR, but it does partially resolve #196
mkavulich added a commit that referenced this pull request Oct 9, 2020
 - Adds a CMake build using ExternalProject_Add.
 - Makes modifications to build_all.sh and install_all.sh for back-compatibility with old build system. The old build system will remain in, but is now deprecated, and will be removed in the coming weeks.

To build, have the necessary modules loaded and run:

```
mkdir build
cd build
cmake .. -DCMAKE_INSTALL_PREFIX=<prefix>
make
```

The docs/ directory contains README files with instructions for specific platforms/compilers

 - **Cheyenne**: Build and end-to-end test ("DOT_OR_USCORE" test case) was successful on Cheyenne with intel, both for the cmake build and the old build script (that will soon be deprecated). Path to tests: /glade/scratch/kavulich/UFS_CAM/testing/SRW_PR_27/expt_dirs/
 - **Hera**: Build and end-to-end tests successful (aside from expected failures). Path to tests: /scratch2/BMC/det/kavulich/workdir/SRW_PR_27/expt_dirs
 - **Jet**: Build test was successful.

Resolves #7, partially resolves #5

Co-authored-by: kgerheiser <[email protected]>; Michael Kavulich, Jr <[email protected]>
mkavulich added a commit that referenced this pull request Oct 9, 2020
## DESCRIPTION OF CHANGES: 
This is a subset of the changes that were added to develop with PR #27. This commit adds cmake capabilities, but does not maintain back-compatibility with the older build system.

To build, have the necessary modules loaded and run:

```
mkdir build
cd build
cmake .. -DCMAKE_INSTALL_PREFIX=<prefix>
make
```

The docs/ directory contains README files with instructions for specific platforms/compilers

## TESTS CONDUCTED: 
 - **Cheyenne**: Build and end-to-end test ("DOT_OR_USCORE" test case) was successful on Cheyenne with intel
 - **Hera**: Build and end-to-end test ("DOT_OR_USCORE" test case) was successful

Co-authored-by: kgerheiser <[email protected]>; Michael Kavulich, Jr <[email protected]>
gsketefian added a commit to ufs-community/regional_workflow that referenced this pull request Oct 15, 2020
…ach repository (#315)

Modifications to run workflow with unmodified executable names from each repository (#304)

## DESCRIPTION OF CHANGES:
The new top-level cmake build for the SRW App ([SRW App PR#27](ufs-community/ufs-srweather-app#27)) results in some executables having different names. This PR makes modifications that
 1. Allow the workflow to run successfully with the new cmake build and its different executable names, and
 2. Allow back-compatibility with the old build system to allow for a gradual transition to new build system

This PR also explicitly disallows running the workflow without CCPP, which we decided against supporting several months ago. I don't think the capability even works so this shouldn't effect anyone at this time
.

## TESTS CONDUCTED:
 - **Cheyenne**: Build and end-to-end test ("DOT_OR_USCORE" test case) was successful on Cheyenne with intel, both for the cmake build and the old build script (that will soon be deprecated).
 - **Hera**: Build and end-to-end tests successful (aside from expected failures). Also built with old build script successfully.
 - **Jet**: Build test was successful.

## ISSUE:
It was not the primary aim of this PR, but it does partially resolve #196
christinaholtNOAA added a commit to christinaholtNOAA/ufs-srweather-app that referenced this pull request Jun 10, 2021
* Updates for inline post.

* Loading upp module. Not building EMC Post

* Removing EMC_post from externals

* Fix AWS build environment settings

Change platform from aws.intel to aws.
Change Intel MPI module to use the module that supports EFA.

* Add workflow env script for AWS

* Fixes for HWT configs.

* Put this file back to original

* Actually put the file back to the original!

* Load rocoto with wflow env.

Co-authored-by: Christopher Harrop <[email protected]>
mkavulich added a commit to mkavulich/ufs-srweather-app that referenced this pull request Aug 26, 2022
…ach repository (ufs-community#304)

## DESCRIPTION OF CHANGES: 
The new top-level cmake build for the SRW App ([SRW App PR#27](ufs-community#27)) results in some executables having different names. This PR makes modifications that
 1. Allow the workflow to run successfully with the new cmake build and its different executable names, and
 2. Allow back-compatibility with the old build system to allow for a gradual transition to new build system

This PR also explicitly disallows running the workflow without CCPP, which we decided against supporting several months ago. I don't think the capability even works so this shouldn't effect anyone at this time.

## TESTS CONDUCTED: 
 - **Cheyenne**: Build and end-to-end test ("DOT_OR_USCORE" test case) was successful on Cheyenne with intel, both for the cmake build and the old build script (that will soon be deprecated). 
 - **Hera**: Build and end-to-end tests successful (aside from expected failures). Also built with old build script successfully.
 - **Jet**: Build test was successful. 

## ISSUE: 
It was not the primary aim of this PR, but it does partially resolve ufs-community#196
natalie-perlin pushed a commit to natalie-perlin/ufs-srweather-app that referenced this pull request Jun 2, 2024
…id for sea ice dynamics (was #1390) (#1381)

* update ice_in and submodules

* cesm_ponds is deprecated so remove from ice_in

* fix hafs-wav configurations

* turn ice coupling off for all hafs+wav tests
* turn current coupling off for hafs-atm-wav test

* add a cdeps test using c-grid for CICE6 (ufs-community#27)
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.

Use cmake build for EMC_post
5 participants