-
Notifications
You must be signed in to change notification settings - Fork 249
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 support for cmake build system #14
Add support for cmake build system #14
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only a partial review, I am still working on it / testing. But I wanted to let you know where I am. Besides the two comments below, I am getting compilation errors for the IPD cmake version on macOS+GNU with the following flags:
./compile_cmake.sh $PWD/.. macosx.gnu 'OPENMP=N 32BIT=Y' '32bit' 2>&1 | tee compile_ipd_new.log
The errors are of this type:
/Users/heinzeller-d/TMP_NOAA/scratch/ufs-weather-model/ufs-weather-model-cmake-rt-for-develop-branch-20191208/compile/ipd_new/FV3/gfsphysics/physics/aer_cloud.F:3173:13:
3173 | delw0=cubicint_ice(SW, SW0, 1d0, 0d0, 1d0)
| 1
Error: Type mismatch in argument 'y2' at (1); passed REAL(16) to REAL(8)
I hope I have time to fix this tonight and propose changes. This problem doesn't occur for CCPP, the following compile commands I do get an executable (didn't run it yet; note also that the OPENMP=N is required on the particular macbook I am currently on, it runs the latest macOS Catalina and this version messed up the OpenMP libraries):
./compile.sh $PWD/../FV3 macosx.gnu 'OPENMP=N SION=Y 32BIT=Y' '32bit' 2>&1 | tee compile_ipd_old.log
./compile.sh $PWD/../FV3 macosx.gnu 'OPENMP=N SION=Y 32BIT=Y CCPP=Y' '32bit' 2>&1 | tee compile_ccpp_dynamic_old.log
./compile.sh $PWD/../FV3 macosx.gnu 'OPENMP=N SION=Y 32BIT=Y CCPP=Y STATIC=Y SUITES=FV3_GFS_2017_gfdlmp,FV3_GSD_v0' '32bit' 2>&1 | tee compile_ccpp_static_old.log
./compile_cmake.sh $PWD/.. macosx.gnu 'OPENMP=N 32BIT=Y CCPP=Y STATIC=Y SUITES=FV3_GFS_2017_gfdlmp,FV3_GSD_v0' '32bit' 2>&1 | tee compile_ccpp_static_new.log
The cmake CCPP dynamic new fails as expected (because we don't support it):
./compile_cmake.sh $PWD/.. macosx.gnu 'OPENMP=N 32BIT=Y CCPP=Y' '32bit' 2>&1 | tee compile_ccpp_dynamic_new.log
I will also try the 64bit versions tonight, as well as DEBUG and REPRO mode, and run all possible executables for at least one setup (GFDL-MP basic setup).
@@ -41,3 +41,14 @@ module load esmf/8.0.0_bs50 | |||
|
|||
# Needed at runtime: | |||
module load alps | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this block be the same as in modulefiles/gaea.intel/fv3?
setenv CMAKE_CXX_COMPILER CC | ||
setenv CMAKE_Fortran_COMPILER ftn | ||
setenv CMAKE_Platform gaea | ||
setenv NETCDF ${NETCDF_DIR} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a line setenv CMAKE_Platform gaea.intel
here?
@DusanJovic-NOAA can you please apply this patch to the fv3atm submodule?
With this, the compile_cmake.sh works on macOS+GNU for IPD as well. Thanks! Also: what do you think of changing
in
automatically if |
Added "fdefault-double-8" to gfsphysics/CMakeLists.txt in b0c4740. |
Thanks for making the change. Agreed, VERBOSE is not a pressing change to make. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this PR on macOS with both gcc+gfortran and clang+gfortran for a range of MAKEOPTS (32bit, 64bit) in DEBUG mode without OpenMP for IPD, CCPP dynamic, CCPP static, both old make and new cmake build. All versions I tested thus far compile and give b4b identical results between IPD and the two CCPP builds, also between gcc+gfortran and clang+gfortran. While I will keep testing with OpenMP and in REPRO and PROD mode, I think this PR is good to go.
Edit: the test I ran were the basic fv3_gfdlmp/fv3_ccpp_gfdlmp regression tests.
Once this is merged, I will update dtc/develop and start to prepare for the commit to develop/master for Dec 20. I did more tests with OpenMP being turned on on macOS for gcc+gfortran, and the results look good. |
Dom,
So you plan to combine these changes with CCPP changes for coupled system?
…On Tue, Dec 10, 2019 at 10:13 PM Dom Heinzeller ***@***.***> wrote:
Once this is merged, I will update dtc/develop and start to prepare for
the commit to develop/master for Dec 20.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#14?email_source=notifications&email_token=AI7D6TNXXTBIJ3QTSQLTSPTQYBLEFA5CNFSM4JXDTBZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGRXJXI#issuecomment-564360413>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AI7D6TKOUFN5WKIGKD6RRLDQYBLEFANCNFSM4JXDTBZQ>
.
|
Yes, I owe you a detailed description of what the PR contains, and I will do this as I create the PRs tomorrow and probably Thursday. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
add dry mass fixer for IAU
…_init Merge in develop of UFS and point to updated WW3
Currently the workflow fails on hera at the make_ics and/or make_lbcs step because the wgrib2 module is not loaded. A PR was accepted in regional_workflow to fix this problem, but the hash was never updated here in the top-level app.
Gaea machine name
No description provided.