update cmake build for testing, build types and mixed precision #1694
Conversation
|
@mathomp4 @aerorahul @dkokron Just wanted to make you aware of these updates and hear any thoughts. We're trying to move towards CMake as our "main" supported build system. |
climbfuji
left a comment
climbfuji
left a comment
There was a problem hiding this comment.
High-level review. Didn't run any tests myself, but CI tests all pass and not seeing any red flags either.
| run: ./test.x | ||
| mkdir build | ||
| cd build | ||
| cmake $CMAKE_FLAGS -DNetCDF_ROOT=/opt/view -DLIBYAML_ROOT=/opt/view .. |
There was a problem hiding this comment.
Using spack containers, nice!
Co-authored-by: Dom Heinzeller <[email protected]>
rem1776
changed the title
uramirez8707
left a comment
uramirez8707
left a comment
There was a problem hiding this comment.
We may need to update the documentation:
https://github.com/rem1776/FMS/blob/f12f78b06b3aab4f16bdbbe746893821a7860f3a/CMAKE_INSTRUCTIONS.md
| run: ./test.x | ||
| mkdir build | ||
| cd build | ||
| cmake $CMAKE_FLAGS -DNetCDF_ROOT=/opt/view -DLIBYAML_ROOT=/opt/view .. |
There was a problem hiding this comment.
Is this running the tests?
|
@uramirez8707 @vithikashah001 @bensonr This PR should be ready now, I just updated the compiler flags based off the meeting discussion. |
uramirez8707
left a comment
uramirez8707
left a comment
There was a problem hiding this comment.
This is looks good to me
| -DOPENMP "Build FMS with OpenMP support" DEFAULT: OFF | ||
| -D32BIT "Build 32-bit (r4) FMS library" DEFAULT: ON | ||
| -DOPENMP "Build FMS with OpenMP support" DEFAULT: ON | ||
| -D32BIT "Build 32-bit (r4) FMS library" DEFAULT: OFF |
There was a problem hiding this comment.
enabling -D64BIT=ON made my cmake configuration fail because the tests were trying to find FMS::fms and it was not built. I can patch this in my PR #1743 to just either not build the tests or fail
| # GNU Fortan | ||
| set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -fcray-pointer -fconvert=big-endian -ffree-line-length-none -fno-range-check -fbacktrace") | ||
| # GNU Fortran | ||
| set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -fcray-pointer -fallow-argument-mismatch -ffree-line-length-none") |
There was a problem hiding this comment.
note here: -fallow-argument-mismatch was introduced in GNU 10.x ish so if anyone tries anything below that, the compilation will fail.
| @ONLY) | ||
|
|
||
| # build any helper modules that are used by unit tests | ||
| list(APPEND TEST_MODS_SRC |
There was a problem hiding this comment.
I know this has been merged already, just keeping the convo from the other PR going with a question here: is there a particular reason why all the files get specified from the top level dir downwards instead of using include_subdirectory and inside each subdir adding the files into the TARGET created?
|
btw this is great, thanks so much for doing this! |
5 participants
Description
This pr updates the cmake build in an effort to behave more similar to autotools, and makes the following changes:
Changes the default value for QUAD_PRECISION and OpenMP options (to off and on, respectively)
Renames the "Release" and "Debug" build type flags to "ReleaseUFS" and "DebugUFS"
Simplifies Release/Debug build types to just add optimization flags and debug info:
Release ->
-O3Debug ->
-O0 -gupdates how mixed precision is handled. Previously a kind type had to be provided when cmake is run, but now if no kind is given it will just build one 8 byte real default version of the library (which includes support for 4 byte reals for most routines as well) and not include the kind size in the built library name. If the kind size is specified, it will behave as it has before for backwards compatibility.
Fixes an invalid llvm intel flag for the C compiler (Fixes Intel LLVM Debug Flags have incorrect flag (for icx 2025.1 at least) #1687)
Adds unit testing via ctest
Updates markdown files for testing and the cmake build instructions
How Has This Been Tested?
cmake + make test
Checklist:
make distcheckpasses