Add CMake build to WW3 #533

kgerheiser · 2021-11-15T15:07:11Z

Pull Request Summary

PR to add a CMake build to WW3

Description

The CMake build handles the build a bit differently than the current build. It builds all available files/executables it can.

The build starts with a base set of files that have no dependencies (see https://github.com/kgerheiser/WW3/blob/feature/cmake/model/src/cmake/src_list.cmake) and then based on switches adds more files to the library (i.e. SCRIP, SCRIPNC, etc). It builds up a ww3_lib library and then links each executable to that library.

The switch file options been extracted from the build system into model/bin/switches.json, which CMake parses at build time to check for necessary files/conflicts. This is meant to be a self-documenting way to add switches. Any new switches only have to be updated in this one location.

See: https://github.com/kgerheiser/WW3/blob/feature/cmake/model/bin/switches.json

Each category is an element in the JSON array which contains a name, description, type, and an array of valid options.

Each valid option object can contain an array build files associated with the given switch. It also has an optional requires and conflicts member to handle conflicting required and conflicting switches. Elements in requires: can be either a string or an array. If a string then that switch is required, if an array then one of those elements is required.

To build:

mkdir build && cd build
cmake .. -DSWITCH=<switch_file> -DCMAKE_INSTALL_PREFIX=install 
make -jn
make install

The build type can be set with -DCMAKE_BUILD_TYPE=<type> where type can be Debug or Release (case-sensitive)

Based on the switches
This will put all executables in the install directory under bin and the library in lib.

Issue(s) addressed

Fixes #139

Check list

Is your feature branch up to date with the authoritative repository (NOAA/develop)?

Yes

Please list appropriate labels code managers should add for this PR:
documentation, enhancement, new feature, ..
Reviewers: @mentions of suggested reviewers of the proposed changes
@JessicaMeixner-NOAA

Commit Message

Add a CMake build to WW3

CMake provides a portable and standardized build system and out-of-source builds. This means a faster and simpler build system. See README for more documentation.

Testing

Testing has been done on Hera and Datarmor, and the build contains a Github Actions build to test on each PR/Push.

…e/cmake

…all, and conflicts

knint is not portable and the generic routine nint should be used instead

…o properly pass them from CMake

CMakeLists.txt

model/bin/switches.json

VERSION

model/bin/w3_setup

edwardhartnett · 2021-11-15T15:48:30Z

model/src/CMakeLists.txt

+math(EXPR len "${len} - 1")
+
+# Loop over switch categories
+set(switch_files "")


You have a clever solution to a complex problem, but I suspect the problem is more complex than it needs to be. It's unlikely that all these compiler switches are really needed. Most packages get by without anything like this level of complexity of build switches.

I would suggest that the coding team not get too comfortable with the JSON solution, and instead seek to reduce the number of switches to a manageable level (less than 5 or 6), and then handle them in the usual way in CMake. This will take some time, no doubt.

In the meantime, there should be very strict review of attempts to add more switches.

Fair enough @edwardhartnett but we need to be able to compile the code as it is now, in all its complexity. The solution @kgerheiser came up with is greatly appreciated.

I agree @edwardhartnett. It's a very unusual build with a lot of complexity, but I didn't want to touch any code, and let the build compile as is.

I totally agree that @kgerheiser cannot resolve these issues, and the build has to cope with them right now. His solution is elegant.

It's an unfortunate side effect of Kyle's clever solution that it's now easier than ever for the development team to add new switches. However, as we have seen, switches cause build system complexity and test difficulties. Better that if-statements are used in the code then #ifdef in the pre-processor, except for certainly well-understood cases, like whether or not a library is present at build-time.

@JessicaMeixner-NOAA perhaps you should consider some overall configuration namelist that can be read at application startup and can carry the switch settings for those switches that can be handled by code, instead of by the build.

Ideally, at some future point, Kyle's JSON solution could be removed, and the remaining switches handled by CMake command line switches, in the typical manner.

Yes the switch file has become a proxy to create multiple combinations of possibilities within the wave model without a review of it being needed or not. The wave model will have to carry out a longer term refactoring/restrategizing on reducing the number of switch options. That will probably take a while :)

kgerheiser · 2022-02-18T15:37:15Z

@mickaelaccensi my develop was out of date. Do a git fetch and try again.

JessicaMeixner-NOAA · 2022-02-18T15:39:07Z

@mickaelaccensi you moved the prep.sh files in ww3_tp2.14 in different directories and both this branch and PR changed them. Do we just need to move the prep.sh in this branch to the new locations? Or are there some of the changes you made that also need to be included, etc?

mickaelaccensi · 2022-02-18T15:47:05Z

ok I'm solving the conflicts then I try to make a PR

mickaelaccensi · 2022-02-18T16:38:34Z

@kgerheiser , I still can't make a PR on your github branch. It seems your visibility is not fully public for PR or contribution...
Because I can select a lot of people github repos but I can't find yours

kgerheiser · 2022-02-18T16:48:27Z

@mickaelaccensi I'm not sure what that's about. I added you as a collaborator on the repo.

mickaelaccensi · 2022-02-18T16:49:57Z

@mickaelaccensi you moved the prep.sh files in ww3_tp2.14 in different directories and both this branch and PR changed them. Do we just need to move the prep.sh in this branch to the new locations? Or are there some of the changes you made that also need to be included, etc?
@JessicaMeixner-NOAA , if you can solve the conflicts, you just need to delete prep_env.sh from ww3_tp2.14/input directory.

JessicaMeixner-NOAA · 2022-02-18T16:56:00Z

@mickaelaccensi you moved the prep.sh files in ww3_tp2.14 in different directories and both this branch and PR changed them. Do we just need to move the prep.sh in this branch to the new locations? Or are there some of the changes you made that also need to be included, etc?
@JessicaMeixner-NOAA , if you can solve the conflicts, you just need to delete prep_env.sh from ww3_tp2.14/input directory.

But @kgerheiser made changes to prep_env.sh to work with cmake. Then you also changed the file itself + changed its locations. I don't know if I should just take the prep_evn.sh in this branch and move it to each of the files or if there needs to be some sort of changes. If you can tell me that I can make the changes, but I don't know the answer to this question to do it myself.

mickaelaccensi · 2022-02-18T16:56:04Z

@mickaelaccensi I'm not sure what that's about. I added you as a collaborator on the repo.

it works ! thanks

…o feature/cmake

Updates for merging tp2.14 updates from develop branch

JessicaMeixner-NOAA · 2022-02-22T14:06:20Z

@kgerheiser I'm starting the final set of regression tests for this and hope to post the results at the end of today and then be able to merge this tomorrow. If there is any needed updates to the PR description, we should do those today.

JessicaMeixner-NOAA · 2022-02-24T14:22:10Z

When comparing this branch to itself, we get the following diff:

**********************************************************************
********************* non-identical cases ****************************
**********************************************************************
mww3_test_03/./work_PR2_UQ_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c                     (6 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR1_MPI_d2                     (10 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c                     (8 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2                     (9 files differ)
ww3_ta1/./work_UPD0F_U                     (0 files differ)
ww3_tp2.10/./work_MPI_OMPH                     (4 files differ)
ww3_tp2.16/./work_MPI_OMPH                     (2 files differ)
ww3_ufs1.3/./work_a                     (1 files differ)

Full matrix:

matrixCompFull.txt
matrixCompSummary.txt
matrixDiff.txt

JessicaMeixner-NOAA · 2022-02-24T14:28:33Z

When comparing this branch to develop,
matrixCompSummary.txt
matrixCompFull.txt
we get a few more diffs (ignoring all 0 files diff):

**********************************************************************
********************* non-identical cases ****************************
**********************************************************************
mww3_test_03/./work_PR2_UQ_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c                     (7 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2                     (6 files differ)
mww3_test_03/./work_PR1_MPI_d2                     (9 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c                     (8 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2                     (8 files differ)
ww3_tp2.10/./work_MPI_OMPH                     (4 files differ)
ww3_tp2.14/./work_OASACM4                     (6 files differ)
ww3_tp2.14/./work_OASICM                     (2 files differ)
ww3_tp2.14/./work_OASOCM                     (9 files differ)
ww3_tp2.14/./work_OASACM2                     (6 files differ)
ww3_tp2.14/./work_OASACM5                     (6 files differ)
ww3_tp2.14/./work_OASACM                     (6 files differ)
ww3_tp2.16/./work_MPI_OMPH                     (16 files differ)
ww3_tp2.5/./work_PR1_MPI                     (5 files differ)
ww3_tp2.5/./work_PR1                     (5 files differ)
ww3_ufs1.3/./work_a                     (1 files differ)

The diffs in tp2.5 are "good" diffs:

/scratch2/NCEPDEV/climate/Jessica.Meixner/PR_WW3/pr533/regtests/output/ww3_tp2.5/work_PR1/ww3.08052210.hs_diff.txt
***
16082c16082
<         -999        -999        -999           0        -999        -999
---
>         -999        -999        -999 -2147483648        -999        -999

in which we get a 0 instead of -2147483648. The tp2.14 tests are likely because a different flag is used in compilation, but we get the same answers on those tests moving forward, so this seems likely okay. The matrixDiff.txt file will not upload for whatever reason.

aliabdolali · 2022-02-24T14:31:32Z

When comparing this branch to develop, matrixCompSummary.txt matrixCompFull.txt we get a few more diffs (ignoring all 0 files diff):

**********************************************************************
********************* non-identical cases ****************************
**********************************************************************
mww3_test_03/./work_PR2_UQ_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c                     (7 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2                     (6 files differ)
mww3_test_03/./work_PR1_MPI_d2                     (9 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c                     (8 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2                     (8 files differ)
ww3_tp2.10/./work_MPI_OMPH                     (4 files differ)
ww3_tp2.14/./work_OASACM4                     (6 files differ)
ww3_tp2.14/./work_OASICM                     (2 files differ)
ww3_tp2.14/./work_OASOCM                     (9 files differ)
ww3_tp2.14/./work_OASACM2                     (6 files differ)
ww3_tp2.14/./work_OASACM5                     (6 files differ)
ww3_tp2.14/./work_OASACM                     (6 files differ)
ww3_tp2.16/./work_MPI_OMPH                     (16 files differ)
ww3_tp2.5/./work_PR1_MPI                     (5 files differ)
ww3_tp2.5/./work_PR1                     (5 files differ)
ww3_ufs1.3/./work_a                     (1 files differ)

The diffs in tp2.5 are "good" diffs:

/scratch2/NCEPDEV/climate/Jessica.Meixner/PR_WW3/pr533/regtests/output/ww3_tp2.5/work_PR1/ww3.08052210.hs_diff.txt
***
16082c16082
<         -999        -999        -999           0        -999        -999
---
>         -999        -999        -999 -2147483648        -999        -999

in which we get a 0 instead of -2147483648. The tp2.14 tests are likely because a different flag is used in compilation, but we get the same answers on those tests moving forward, so this seems likely okay. The matrixDiff.txt file will not upload for whatever reason.

When the matrixDiff is very large, github does not allow you to upload it.

JessicaMeixner-NOAA · 2022-02-24T14:39:56Z

@aliabdolali good to know. I'm happy to post any portion of the matrixdiff if anyone is curious.

fix matrix.comp

aliabdolali · 2022-02-24T14:56:20Z

thanks @kgerheiser for this great development.

arunchawla-NOAA · 2022-02-24T14:57:22Z

hurrah !!!!!!!

edwardhartnett · 2022-02-28T13:47:48Z

Good work @kgerheiser , this was a lot more complex than I anticipated!

CMake provides a portable and standardized build system and out-of-source builds. This means a faster and simpler build system. See README for more documentation.

kgerheiser added 23 commits October 21, 2021 16:09

Ignore .DS_Store

233175e

Create first CMakeLists.txt and associated files

573fbb6

Create JSON file to hold switch information

ce6b375

Merge branch 'develop' of https://github.com/NOAA-EMC/WW3 into featur…

21902e7

…e/cmake

Convert to array of objects

b1c2dd9

Finish filling out and make valid-options an array

b74de4e

Formatting

ddf6851

Move switches.json into bin

4800678

Open and read switches.json from CMake

14f254c

Ignore build directory

4b5962a

Fix some switches

c377818

Add FindMETIS/FindParMETIS modules

32be750

Add CMake functionality

47f003c

Handle NCEP2, PDLIB, and SCRIP switch libraries

652bc7d

Fix typo in filename

2248199

Add switch dependencies and conflicts through requires_any, requires_…

ed5c4b3

…all, and conflicts

Use generic routine nint instead of Intel extension knint

5775465

knint is not portable and the generic routine nint should be used instead

Add checks for conflicting and required dependent switches

eb4fd5d

Add CMake package config

186a0d1

Document CMake build

bc42b4a

Move FindESMF into SCRIP block

2d3501d

Comment out list switches string in w3initmd while I figure out how t…

025d685

…o properly pass them from CMake

Cleanup switches.json

a70b317

edwardhartnett reviewed Nov 15, 2021

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

edwardhartnett reviewed Nov 15, 2021

View reviewed changes

model/bin/switches.json Outdated Show resolved Hide resolved

JessicaMeixner-NOAA reviewed Nov 15, 2021

View reviewed changes

VERSION Outdated Show resolved Hide resolved

JessicaMeixner-NOAA reviewed Nov 15, 2021

View reviewed changes

model/bin/w3_setup Outdated Show resolved Hide resolved

edwardhartnett reviewed Nov 15, 2021

View reviewed changes

aliabdolali self-requested a review November 15, 2021 16:07

Revert changes to w3_setup

023e030

Merge branch 'develop' into cmake

f4de739

mickaelaccensi and others added 5 commits February 18, 2022 17:30

update prep_env and CMakeLists.txt for cmake compilation

ddcbb13

update run_cmake_test

f1a7239

make matrix.comp work

a2f0bf0

Merge branch 'feature/cmake' of https://github.com/kgerheiser/WW3 int…

538dfad

…o feature/cmake

Merge pull request #9 from JessicaMeixner-NOAA/feature/cmake

facd263

Updates for merging tp2.14 updates from develop branch

fix matrix.comp

c6f2906

Merge pull request #10 from JessicaMeixner-NOAA/feature/cmake

df7a663

fix matrix.comp

aliabdolali approved these changes Feb 24, 2022

View reviewed changes

JessicaMeixner-NOAA approved these changes Feb 24, 2022

View reviewed changes

JessicaMeixner-NOAA merged commit cfcd089 into NOAA-EMC:develop Feb 24, 2022

kgerheiser mentioned this pull request Feb 24, 2022

Fix ufs-weather-model integration with CMake #627

Closed

DeniseWorthen mentioned this pull request Mar 14, 2022

CMAKE for WW3 ufs-community/ufs-weather-model#1089

Merged

16 tasks

aliabdolali pushed a commit that referenced this pull request Mar 16, 2022

Add CMake build to WW3 (#533)

a4112a0

CMake provides a portable and standardized build system and out-of-source builds. This means a faster and simpler build system. See README for more documentation.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CMake build to WW3 #533

Add CMake build to WW3 #533

kgerheiser commented Nov 15, 2021 •

edited

Loading

edwardhartnett Nov 15, 2021

JessicaMeixner-NOAA Nov 15, 2021

kgerheiser Nov 15, 2021

edwardhartnett Nov 15, 2021

arunchawla-NOAA Nov 15, 2021

kgerheiser commented Feb 18, 2022

JessicaMeixner-NOAA commented Feb 18, 2022

mickaelaccensi commented Feb 18, 2022

mickaelaccensi commented Feb 18, 2022

kgerheiser commented Feb 18, 2022

mickaelaccensi commented Feb 18, 2022

JessicaMeixner-NOAA commented Feb 18, 2022

mickaelaccensi commented Feb 18, 2022

JessicaMeixner-NOAA commented Feb 22, 2022

JessicaMeixner-NOAA commented Feb 24, 2022

JessicaMeixner-NOAA commented Feb 24, 2022

aliabdolali commented Feb 24, 2022

JessicaMeixner-NOAA commented Feb 24, 2022

aliabdolali commented Feb 24, 2022

arunchawla-NOAA commented Feb 24, 2022

edwardhartnett commented Feb 28, 2022

Add CMake build to WW3 #533

Add CMake build to WW3 #533

Conversation

kgerheiser commented Nov 15, 2021 • edited Loading

Pull Request Summary

Description

Issue(s) addressed

Check list

Commit Message

Testing

edwardhartnett Nov 15, 2021

Choose a reason for hiding this comment

JessicaMeixner-NOAA Nov 15, 2021

Choose a reason for hiding this comment

kgerheiser Nov 15, 2021

Choose a reason for hiding this comment

edwardhartnett Nov 15, 2021

Choose a reason for hiding this comment

arunchawla-NOAA Nov 15, 2021

Choose a reason for hiding this comment

kgerheiser commented Feb 18, 2022

JessicaMeixner-NOAA commented Feb 18, 2022

mickaelaccensi commented Feb 18, 2022

mickaelaccensi commented Feb 18, 2022

kgerheiser commented Feb 18, 2022

mickaelaccensi commented Feb 18, 2022

JessicaMeixner-NOAA commented Feb 18, 2022

mickaelaccensi commented Feb 18, 2022

JessicaMeixner-NOAA commented Feb 22, 2022

JessicaMeixner-NOAA commented Feb 24, 2022

JessicaMeixner-NOAA commented Feb 24, 2022

aliabdolali commented Feb 24, 2022

JessicaMeixner-NOAA commented Feb 24, 2022

aliabdolali commented Feb 24, 2022

arunchawla-NOAA commented Feb 24, 2022

edwardhartnett commented Feb 28, 2022

kgerheiser commented Nov 15, 2021 •

edited

Loading