Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions .github/workflows/github_cmake_gnu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,46 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
omp-flags: [ -DOPENMP=on, -DOPENMP=off ]
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious, why was this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was mainly to make the amount of jobs smaller. It'll just compile with openmp everytime.

I was thinking as we move away from autotools we can add more options to this one, just didn't want to make the CI too bloated.

libyaml-flag: [ "", -DWITH_YAML=on ]
build-type: [ "-DCMAKE_BUILD_TYPE=Release", "-DCMAKE_BUILD_TYPE=Debug" ]
container:
image: ghcr.io/noaa-gfdl/fms/fms-ci-rocky-gnu:13.2.0
env:
CMAKE_FLAGS: "${{ matrix.build-type }} ${{ matrix.omp-flags }} ${{ matrix.libyaml-flag }}"
CMAKE_FLAGS: "${{ matrix.build-type }} ${{ matrix.libyaml-flag }}"
EXCLUDE_TESTS: "test_mpp_nesting"
steps:
- name: Checkout code
uses: actions/checkout@v4.2.2
- name: Generate makefiles with CMake
run: |
mkdir build
cd build
cmake $CMAKE_FLAGS -DNetCDF_ROOT=/opt/view -DLIBYAML_ROOT=/opt/view ..
cmake $CMAKE_FLAGS -DOPENMP=on -DNetCDF_ROOT=/opt/view -DLIBYAML_ROOT=/opt/view ..
- name: Build the library
run: make -C build
- name: Run the unit tests
run: cd build && ctest -E "${EXCLUDE_TESTS}" --output-on-failure

build_arm:
runs-on: ubuntu-24.04-arm
strategy:
matrix:
omp-flags: [ -DOPENMP=on, -DOPENMP=off ]
libyaml-flag: [ "", -DWITH_YAML=on ]
build-type: [ "-DCMAKE_BUILD_TYPE=Release", "-DCMAKE_BUILD_TYPE=Debug" ]
container:
image: ghcr.io/noaa-gfdl/fms/fms-ci-rocky-gnu:13.2.0-arm
env:
CMAKE_FLAGS: "${{ matrix.build-type }} ${{ matrix.omp-flags }} ${{ matrix.libyaml-flag }}"
CMAKE_FLAGS: "${{ matrix.build-type }} ${{ matrix.libyaml-flag }}"
EXCLUDE_TESTS: ${{ matrix.build-type == '-DCMAKE_BUILD_TYPE=Release' && 'test_mpp_nesting|test_sat_vapor_pres' || 'test_mpp_nesting' }}
steps:
- name: Checkout code
uses: actions/checkout@v4.2.2
- name: Generate makefiles with CMake
run: |
mkdir build
cd build
cmake $CMAKE_FLAGS -DNetCDF_ROOT=/opt/view -DLIBYAML_ROOT=/opt/view ..
cmake $CMAKE_FLAGS -DOPENMP=on -DNetCDF_ROOT=/opt/view -DLIBYAML_ROOT=/opt/view ..
- name: Build the library
run: make -C build
- name: Run the unit tests
run: cd build && ctest -E "${EXCLUDE_TESTS}" --output-on-failure
49 changes: 34 additions & 15 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ foreach(kind ${kinds})
amip_interp/include)
target_compile_definitions(${libTgt}_f PRIVATE "${fms_defs}")
target_compile_definitions(${libTgt}_f PRIVATE "${${kind}_defs}")
set_target_properties(${libTgt}_f PROPERTIES COMPILE_FLAGS ${${kind}_flags})
set_target_properties(${libTgt}_f PROPERTIES COMPILE_FLAGS "${${kind}_flags}")
Copy link
Contributor

@mlee03 mlee03 Oct 23, 2025

Choose a reason for hiding this comment

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

comments not for this line, but just looking at line 76 - 79, the messages seem misleading. Could they be changed to something like "building select modules and procedures in 32bit precision"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, line 278 - 278, I don't think the OVERLOAD macros are there anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, line 278 - 278, I don't think the OVERLOAD macros are there anymore

nice catch! should be removed now

set_target_properties(${libTgt}_f PROPERTIES Fortran_MODULE_DIRECTORY
${moduleDir})
target_link_libraries(${libTgt}_f PRIVATE NetCDF::NetCDF_Fortran
Expand Down Expand Up @@ -588,6 +588,14 @@ include(CTest)
set(MPI_LAUNCHER "mpirun")
# used in the test-lib.sh.in to make it behave differently when parsed by cmake
set(USING_CMAKE "true")
# set the fms library to link tests with based on whats built
if(NOT kinds)
set(fmsLibraryName FMS::fms)
Copy link
Contributor

Choose a reason for hiding this comment

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

line 439, isn't kind always defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

line 439 uses kind as opposed to kinds which is only set if one of the 32/64bit options is enabled.

elseif(64BIT)
set(fmsLibraryName FMS::fms_r8)
else()
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this should explicitly be elseif(32BIT)

set(fmsLibraryName FMS::fms_r4)
endif()

# parse and add build info to test script util file
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/test_fms/test-lib.sh.in ${CMAKE_CURRENT_SOURCE_DIR}/test-lib.sh
Expand All @@ -610,7 +618,7 @@ list(APPEND TEST_MODS_SRC
test_fms/mpp/test_system_clock.F90)

add_library(testLibs "${TEST_MODS_SRC}")
target_link_libraries(testLibs FMS::fms)
target_link_libraries(testLibs ${fmsLibraryName})
target_compile_definitions(testLibs PRIVATE TEST_MOS_KIND_=r8_kind)
target_include_directories(testLibs PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/include
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/test_fms/fms/include
Expand Down Expand Up @@ -691,6 +699,7 @@ list(APPEND TEST_SRC_SINGLE_TARGET
test_fms/fms2_io/test_domain_io.F90
test_fms/fms2_io/test_collective_io.F90
test_fms/fms2_io/test_generalized_indices.F90
test_fms/fms2_io/test_parallel_writes.F90
test_fms/fms/test_fms.F90
test_fms/interpolator/test_interpolator.F90
test_fms/mpp/test_clock_init.F90
Expand Down Expand Up @@ -769,21 +778,25 @@ list(APPEND TEST_SRC_MULTI_TARGET
test_fms/sat_vapor_pres/test_sat_vapor_pres.F90
test_fms/time_interp/test_time_interp.F90
test_fms/time_interp/test_time_interp_external.F90
test_fms/time_interp/test_time_interp_conservative_hi.F90
test_fms/topography/test_topography.F90
test_fms/tracer_manager/test_tracer_manager.F90
test_fms/tridiagonal/test_tridiagonal.F90
)

# add all the test executables and set up linking + compile flags
foreach (testFile ${TEST_SRC_SINGLE_TARGET})
get_filename_component (TName ${testFile} NAME_WE)

get_filename_component (TName ${testFile} NAME_WE)
add_executable(${TName} ${testFile})
target_compile_definitions(${TName} PRIVATE "${r8_defs}")
set_target_properties(${TName} PROPERTIES COMPILE_FLAGS ${r8_flags})
target_link_libraries(${TName} PUBLIC FMS::fms
PRIVATE testLibs
)
target_link_libraries(${TName} PUBLIC ${fmsLibraryName}
PRIVATE testLibs)
if(32BIT)
target_compile_definitions(${TName} PRIVATE "${r4_defs} ${fms_defs}")
else()
target_compile_definitions(${TName} PRIVATE "${r8_defs} ${fms_defs} ")
set_target_properties(${TName} PROPERTIES COMPILE_FLAGS ${r8_flags})
endif()

if(WITH_YAML)
target_link_libraries(${TName} PRIVATE yaml ${LIBYAML_LIBRARIES})
Expand All @@ -806,18 +819,16 @@ foreach (testFile ${TEST_SRC_MULTI_TARGET})
get_filename_component (TName ${testFile} NAME_WE)

add_executable(${TName}_r8 ${testFile})
target_compile_definitions(${TName}_r8 PRIVATE "${r8_defs}")
target_compile_definitions(${TName}_r8 PRIVATE "${r8_defs} ${fms_defs}")
set_target_properties(${TName}_r8 PROPERTIES COMPILE_FLAGS ${r8_flags})
target_link_libraries(${TName}_r8 PUBLIC FMS::fms
PRIVATE testLibs
)

target_link_libraries(${TName}_r8 PUBLIC ${fmsLibraryName}
PRIVATE testLibs)
add_executable(${TName}_r4 ${testFile})
target_compile_definitions(${TName}_r4 PRIVATE "${r4_defs}")
target_compile_definitions(${TName}_r4 PRIVATE "${r4_defs} ${fms_defs}")
# seems counterintuitive but r4 tests use r8 default
# they specify kind values explicitly with the preprocessor where needed (TEST_FMS_KIND_)
set_target_properties(${TName}_r4 PROPERTIES COMPILE_FLAGS ${r8_flags})
target_link_libraries(${TName}_r4 PUBLIC FMS::fms
target_link_libraries(${TName}_r4 PUBLIC ${fmsLibraryName}
PRIVATE testLibs
)

Expand Down Expand Up @@ -879,8 +890,16 @@ foreach (testScript ${TEST_SCRIPTS})
ENVIRONMENT "parser_skip=skip"
)
endif()

endforeach()

# skip parallel netcdf tests if its not enabled in given install
IF(NOT NetCDF_PARALLEL)
Copy link
Contributor

Choose a reason for hiding this comment

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

Leaving what @J-Lentz pointed out, line 290 should say "parallel netcdf io" instead of "collective netcdf io"

set_tests_properties(test_collective_io PROPERTIES
ENVIRONMENT "parallel_skip=skip"
)
endif()

set(CMAKE_CTEST_ARGUMENTS "--output-on-failure")

### Package config
Expand Down
2 changes: 1 addition & 1 deletion cmake/compiler_flags_GNU_Fortran.cmake
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Precision-based Fortran compiler flags
set(r8_flags "-fdefault-real-8 -fdefault-double-8") # Fortran flags for 64BIT precision
set(r4_flags "-fdefault-real-4") # Fortran flags for 32BIT precision
set(r4_flags "") # 4 byte reals is default for gfortran

# GNU Fortran
set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -fcray-pointer -fallow-argument-mismatch -ffree-line-length-none")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ program test_time_interp_with_interp
use time_interp_external2_mod, only: init_external_field, time_interp_external_init, time_interp_external

use time_manager_mod, only: JULIAN, time_type, set_date, set_calendar_type, time_manager_init
use platform_mod, only: r4_kind, r8_kind
Copy link
Contributor

Choose a reason for hiding this comment

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

ah this probably worked because DTEST_FMS_KIND_ is set to 4 or 8 in Makefile.am
Is this use statement needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

unless cmake substitutes them as r4_kind and r8_kind somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unless cmake substitutes them as r4_kind and r8_kind somewhere?

Yeah cmake uses r4/r8_kind so that its uniform across all the different unit tests.


implicit none

Expand Down
Loading