Skip to content

Update to kokkos5 and c++20 #410

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.

Sign up for GitHub

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

Open
tcclevenger wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Update to kokkos5 and c++20 #410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fcfe014
Update minimum version C++17 -> C++20
tcclevenger Nov 24, 2025
fcc6985
Do not set testing to C++20, no need now
tcclevenger Dec 1, 2025
5f6a0b2
Update to Kokkos 5.0.1
tcclevenger Nov 21, 2025
843e87d
Force Kokkos_ENABLE_IMPL_VIEW_LEGACY=ON
tcclevenger Nov 21, 2025
17c8d59
Update spdlog
tcclevenger Jan 27, 2026
52a488e
Fix deprecation warning
tcclevenger Feb 10, 2026
d81f690
Remove -restrict cxx flag (not compatible with oneapi
tcclevenger Feb 11, 2026
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
10 changes: 10 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
cmake_minimum_required(VERSION 3.13)

# EKAT requires C++20
set(CMAKE_CXX_STANDARD 20)
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

set(CMAKE_CXX_STANDARD 20) alone does not strictly require C++20; CMake may still accept older standards if the compiler can’t satisfy it, and may also default to compiler extensions. To truly enforce “require C++20”, also set CMAKE_CXX_STANDARD_REQUIRED to ON and (typically) CMAKE_CXX_EXTENSIONS to OFF near this block.

Suggested change
set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +4
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Setting CMAKE_CXX_STANDARD at the top-level can leak into parent projects when EKAT is consumed via add_subdirectory, effectively changing the consumer’s global C++ standard. Prefer expressing the requirement on EKAT targets (e.g., via target_compile_features(... PUBLIC cxx_std_20) on public libraries) or only setting the global standard when EKAT is the top-level project (guarded with if (CMAKE_SOURCE_DIR STREQUAL PROJECT_SOURCE_DIR)).

Suggested change
# EKAT requires C++20
set(CMAKE_CXX_STANDARD 20)
# EKAT requires C++20 when built as a top-level project
if (CMAKE_SOURCE_DIR STREQUAL PROJECT_SOURCE_DIR)
set(CMAKE_CXX_STANDARD 20)
endif()

Copilot uses AI. Check for mistakes.

set (EKAT_CMAKE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake CACHE INTERNAL "")

# Do not ignore <PackageName_ROOT> env vars in find_package() calls
Expand Down Expand Up @@ -86,6 +89,13 @@ if (EKAT_ENABLE_MPI)
set(EKAT_MPI_THREAD_FLAG "" CACHE STRING "The mpirun flag for designating the number of threads")
endif()

##############################
### KOKKOS CONFIG OPTIONS ###
##############################
# FIXME: View usage in Hommexx and EAMxx needs to allow for Kokkos' MDSpan view implementation as the legacy
# implementation will go away in the future, unnanounced (it's an "impl" option => no deprecation).
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Correct spelling of “unnanounced” to “unannounced”.

Suggested change
# implementation will go away in the future, unnanounced (it's an "impl" option => no deprecation).
# implementation will go away in the future, unannounced (it's an "impl" option => no deprecation).

Copilot uses AI. Check for mistakes.
option (Kokkos_ENABLE_IMPL_VIEW_LEGACY "Currently, EKAT is only compatible with Kokkos' legacy view implementation." ON)
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

As written, this reads like an EKAT-level switch, but it only has an effect if EKAT is configuring/building Kokkos from source in the same CMake build (e.g., via add_subdirectory); it won’t change anything when Kokkos is prebuilt and provided via find_package. Consider clarifying that in the option description (or adding a configure-time note) so users aren’t misled into thinking toggling this will reconfigure an externally-provided Kokkos.

Suggested change
option (Kokkos_ENABLE_IMPL_VIEW_LEGACY "Currently, EKAT is only compatible with Kokkos' legacy view implementation." ON)
option (Kokkos_ENABLE_IMPL_VIEW_LEGACY "Currently, EKAT is only compatible with Kokkos' legacy view implementation. This option only takes effect when EKAT is configuring/building Kokkos from source in the same CMake build (e.g., via add_subdirectory) and does not reconfigure an externally provided Kokkos found via find_package." ON)

Copilot uses AI. Check for mistakes.

############################################
### COMPILER/OS-SPECIFIC CONFIG OPTIONS ###
############################################
Expand Down
1 change: 0 additions & 1 deletion cacts.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ configurations:
uses_baselines: False
on_by_default: True
cmake_args:
CMAKE_CXX_STANDARD: 20
EKAT_ENABLE_ALL_PACKAGES: True
EKAT_TEST_THREAD_INC: ${2 if machine.gpu_arch is None else 1}
EKAT_TEST_MAX_THREADS: ${machine.num_run_res if machine.gpu_arch is None else 1}
Expand Down
2 changes: 1 addition & 1 deletion cmake/EkatSetCompilerFlags.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ endmacro()
#############################
macro (SetGeneralFlags)
if (CMAKE_Fortran_COMPILER_ID STREQUAL "Intel" OR CMAKE_Fortran_COMPILER_ID STREQUAL "IntelLLVM")
SetFlags (FFLAGS "-assume byterecl -ftz" CXXFLAGS -restrict)
SetFlags (FFLAGS "-assume byterecl -ftz")
elseif (CMAKE_Fortran_COMPILER_ID STREQUAL "GNU")
SetFlags (FFLAGS -ffree-line-length-none)
endif()
Expand Down
2 changes: 1 addition & 1 deletion extern/kokkos
Open in desktop
Submodule kokkos updated 1461 files
2 changes: 1 addition & 1 deletion extern/spdlog
Open in desktop
Submodule spdlog updated 134 files
3 changes: 0 additions & 3 deletions src/core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ add_library(ekat_core
ekat_test_utils.cpp
)

Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

This hunk removes the explicit per-target language standard requirement for ekat_core. If the intent is to require C++20, it’s more robust to keep the requirement attached to the target (now as cxx_std_20) so the constraint propagates correctly to consumers of ekat_core and doesn’t rely on a global CMAKE_CXX_STANDARD setting.

Suggested change
target_compile_features(ekat_core PUBLIC cxx_std_20)

Copilot uses AI. Check for mistakes.
# EKAT requires c++17 features
target_compile_features(ekat_core PUBLIC cxx_std_17)

# Add the correct ekat::Comm impl file, depending on whether MPI is ON/OFF.
if (EKAT_ENABLE_MPI)
target_sources(ekat_core PRIVATE ekat_comm.cpp)
Expand Down
2 changes: 1 addition & 1 deletion src/kokkos/ekat_kokkos_meta.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ struct MemoryTraitsMask {
template <typename View>
using Unmanaged =
// Provide a full View type specification, augmented with Unmanaged.
Kokkos::View<typename View::traits::scalar_array_type,
Kokkos::View<typename View::traits::data_type,
typename View::traits::array_layout,
typename View::traits::device_type,
Kokkos::MemoryTraits<
Expand Down