Skip to content
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

Fix CTests in SYCL runs #153

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from
Draft

Fix CTests in SYCL runs #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cd8a9e5
Move -Xsycl-target-backend to link flags
amametjanov Oct 29, 2024
8dd30e2
Fix SYCL and PBS flags
amametjanov Oct 29, 2024
a9c2fd8
Default to PIO_IOTYPE_PNETCDF to work around HDF5 in SYCL runs
amametjanov Oct 29, 2024
be5c90c
Move SYCL link flags to OMEGA_SYCL_EXE_LINKER_FLAGS
amametjanov Dec 11, 2024
e4f899d
Clean-up slurm-specific core binding in a ctest
amametjanov Dec 13, 2024
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
3 changes: 2 additions & 1 deletion cime_config/machines/cmake_macros/oneapi-ifxgpu_aurora.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ string(APPEND CMAKE_EXE_LINKER_FLAGS " -lmkl_intel_lp64 -lmkl_sequential -lmkl_c
if (compile_threaded)
string(APPEND CMAKE_EXE_LINKER_FLAGS " -fiopenmp -fopenmp-targets=spir64")
endif()
string(APPEND SYCL_FLAGS " -\-intel -fsycl -fsycl-targets=spir64_gen -mlong-double-64 -Xsycl-target-backend \"-device 12.60.7\"")
string(APPEND SYCL_FLAGS " -\-intel -fsycl -fsycl-targets=spir64_gen -mlong-double-64 ")
string(APPEND OMEGA_SYCL_EXE_LINKER_FLAGS " -Xsycl-target-backend \"-device 12.60.7\" ")
string(APPEND CMAKE_CXX_FLAGS " -Xclang -fsycl-allow-virtual-functions")
3 changes: 2 additions & 1 deletion cime_config/machines/cmake_macros/oneapi-ifxgpu_sunspot.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ string(APPEND CMAKE_EXE_LINKER_FLAGS " -lmkl_intel_lp64 -lmkl_sequential -lmkl_c
if (compile_threaded)
string(APPEND CMAKE_EXE_LINKER_FLAGS " -fiopenmp -fopenmp-targets=spir64")
endif()
string(APPEND SYCL_FLAGS " -\-intel -fsycl -fsycl-targets=spir64_gen -mlong-double-64 -Xsycl-target-backend \"-device 12.60.7\"")
string(APPEND SYCL_FLAGS " -\-intel -fsycl -fsycl-targets=spir64_gen -mlong-double-64 ")
string(APPEND OMEGA_SYCL_EXE_LINKER_FLAGS " -Xsycl-target-backend \"-device 12.60.7\" ")
string(APPEND CMAKE_CXX_FLAGS " -Xclang -fsycl-allow-virtual-functions")
2 changes: 1 addition & 1 deletion components/omega/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ if (NOT DEFINED PROJECT_NAME)

set(CMAKE_CXX_STANDARD 17) # used in E3SM
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(LINKER_LANGUAGE C) # needed to support Cray C compiler wrapper
set(LINKER_LANGUAGE CXX) # needed to support Cray C compiler wrapper
Copy link

Choose a reason for hiding this comment

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

On Frontier, there was an issue with linking to the CXX compiler because mipcxx was set to CXX instead of the CC Cray compiler wrapper. During the linking phase, the Omega compilation failed. Time have passed since then, and a quick build with this change now seems to pass the linking stage, but please ensure that this change works correctly with all compilers on Frontier.


# update variables for standalone build
setup_standalone_build()
Expand Down
31 changes: 21 additions & 10 deletions components/omega/OmegaBuild.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ macro(common)
option(OMEGA_DEBUG "Turn on error message throwing (default OFF)." OFF)
option(OMEGA_LOG_FLUSH "Turn on unbuffered logging (default OFF)." OFF)

if("${OMEGA_BUILD_TYPE}" STREQUAL "Debug" OR "${OMEGA_BUILD_TYPE}" STREQUAL "DEBUG")
set(OMEGA_DEBUG ON)
endif()

if(NOT DEFINED OMEGA_CXX_FLAGS)
set(OMEGA_CXX_FLAGS "")
endif()
Expand Down Expand Up @@ -120,7 +124,7 @@ macro(read_cime_config)
break()

elseif("${arg}" STREQUAL "-n" OR "${arg}" STREQUAL "-N" OR
"${arg}" STREQUAL "-c")
"${arg}" STREQUAL "-c" OR "${arg}" STREQUAL "-np")
set(SKIP_ARG TRUE)

else()
Expand Down Expand Up @@ -219,6 +223,9 @@ macro(init_standalone_build)
elseif(USE_HIP)
set(OMEGA_ARCH "HIP")

elseif(USE_SYCL)
set(OMEGA_ARCH "SYCL")

else()

execute_process(
Expand Down Expand Up @@ -294,7 +301,11 @@ macro(init_standalone_build)
set(_CtestScript ${OMEGA_BUILD_DIR}/omega_ctest.sh)
file(WRITE ${_CtestScript} "#!/usr/bin/env bash\n\n")
file(APPEND ${_CtestScript} "source ./omega_env.sh\n\n")
file(APPEND ${_CtestScript} "ctest --output-on-failure $* # --rerun-failed\n\n")
if(OMEGA_DEBUG)
file(APPEND ${_CtestScript} "ctest --output-on-failure --verbose $* # --rerun-failed\n\n")
else()
file(APPEND ${_CtestScript} "ctest --output-on-failure $* # --rerun-failed\n\n")
endif()

# create a profile script
set(_ProfileScript ${OMEGA_BUILD_DIR}/omega_profile.sh)
Expand Down Expand Up @@ -422,10 +433,14 @@ macro(init_standalone_build)
file(APPEND ${_ProfileScript} " -o \$OUTFILE ./src/omega.exe 1000")

elseif("${OMEGA_ARCH}" STREQUAL "SYCL")
set(CMAKE_CXX_COMPILER ${OMEGA_SYCL_COMPILER})
set(CMAKE_CXX_COMPILER ${OMEGA_CXX_COMPILER})

if(OMEGA_SYCL_FLAGS)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OMEGA_SYCL_FLAGS}")
# add flags from upstream-E3SM
if(SYCL_FLAGS)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${SYCL_FLAGS}")
Copy link

Choose a reason for hiding this comment

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

We want to add the "OMEGA_" prefix to all Omega-related CMake variables and prefer using "OMEGA_SYCL_FLAGS" instead of "SYCL_FLAGS."

endif()
if(OMEGA_SYCL_EXE_LINKER_FLAGS)
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OMEGA_SYCL_EXE_LINKER_FLAGS}")
endif()

else()
Expand Down Expand Up @@ -458,7 +473,7 @@ macro(init_standalone_build)

message(STATUS "CMAKE_CXX_COMPILER = ${CMAKE_CXX_COMPILER}")
message(STATUS "CMAKE_CXX_FLAGS = ${CMAKE_CXX_FLAGS}")
# message(STATUS "CMAKE_EXE_LINKER_FLAGS = ${CMAKE_EXE_LINKER_FLAGS}")
message(STATUS "CMAKE_EXE_LINKER_FLAGS = ${CMAKE_EXE_LINKER_FLAGS}")

endmacro()

Expand Down Expand Up @@ -512,10 +527,6 @@ macro(update_variables)

add_definitions(-DOMEGA_BUILD_MODE=${OMEGA_BUILD_MODE})

if("${OMEGA_BUILD_TYPE}" STREQUAL "Debug" OR "${OMEGA_BUILD_TYPE}" STREQUAL "DEBUG")
set(OMEGA_DEBUG ON)
endif()

if(NOT DEFINED OMEGA_LOG_LEVEL)
set(OMEGA_LOG_LEVEL "INFO")
endif()
Expand Down
4 changes: 2 additions & 2 deletions components/omega/src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ target_link_libraries(
if(GKlib_FOUND)
target_link_libraries(
OmegaLibFlags
PUBLIC
INTERFACE
gklib
)
endif()
Expand Down Expand Up @@ -106,6 +106,6 @@ if(OMEGA_BUILD_EXECUTABLE)
OmegaLibFlags
)

set_target_properties(${OMEGA_EXE_NAME} PROPERTIES LINKER_LANGUAGE C)
set_target_properties(${OMEGA_EXE_NAME} PROPERTIES LINKER_LANGUAGE CXX)
Copy link

Choose a reason for hiding this comment

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

Same as the above comment about using C linker instead of CXX.


endif()
2 changes: 2 additions & 0 deletions components/omega/src/base/DataTypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ KOKKOS_INLINE_FUNCTION constexpr Real operator""_Real(long double x) {
using MemSpace = Kokkos::CudaSpace;
#elif OMEGA_ENABLE_HIP
using MemSpace = Kokkos::Experimental::HIPSpace;
#elif OMEGA_ENABLE_SYCL
using MemSpace = Kokkos::Experimental::SYCLDeviceUSMSpace;
#elif OMEGA_ENABLE_OPENMP
using MemSpace = Kokkos::HostSpace;
#elif OMEGA_ENABLE_SERIAL
Expand Down
2 changes: 1 addition & 1 deletion components/omega/src/base/IO.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ enum FileFmt {
FmtHDF5 = PIO_IOTYPE_HDF5, ///< native HDF5 format
FmtADIOS = PIO_IOTYPE_ADIOS, ///< ADIOS format
FmtUnknown = -1, ///< Unknown or undefined
FmtDefault = PIO_IOTYPE_NETCDF4C, ///< NetCDF4 is default
FmtDefault = PIO_IOTYPE_PNETCDF, ///< work around HDF5 in SYCL runs
Copy link

Choose a reason for hiding this comment

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

Does this change, setting the default I/O type to PNETCDF, work on all machines, including Frontier, Perlmutter, and Chrysalis?

};

/// File operations
Expand Down
26 changes: 17 additions & 9 deletions components/omega/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,22 @@ function(add_omega_test test_name exe_name source_files mpi_args)
)
endif()

set_target_properties(${exe_name} PROPERTIES LINKER_LANGUAGE C)
set_target_properties(${exe_name} PROPERTIES LINKER_LANGUAGE CXX)
Copy link

Choose a reason for hiding this comment

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

Same as the above comment about using C linker instead of CXX.


# Add the test command
if (mpi_args)
add_test(
NAME ${test_name}
COMMAND ${OMEGA_MPI_EXEC} ${OMEGA_MPI_ARGS} ${mpi_args} -- ./${exe_name}
)

if("${OMEGA_ARCH}" STREQUAL "SYCL")
add_test(
NAME ${test_name}
COMMAND ${OMEGA_MPI_EXEC} ${mpi_args} ${OMEGA_MPI_ARGS} ./${exe_name}
Copy link

Choose a reason for hiding this comment

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

I am curious why the order of MPI arguments matters.

)
else()
add_test(
NAME ${test_name}
COMMAND ${OMEGA_MPI_EXEC} ${OMEGA_MPI_ARGS} ${mpi_args} -- ./${exe_name}
)
endif()

else()
add_test(
Expand Down Expand Up @@ -281,7 +289,7 @@ add_omega_test(
TEND_PLANE_TEST
testTendencyTermsPlane.exe
ocn/TendencyTermsTest.cpp
"-n 8;--cpu-bind=cores"
"-n 8;"
)
target_compile_definitions(
testTendencyTermsPlane.exe
Expand All @@ -293,15 +301,15 @@ add_omega_test(
TEND_PLANE_SINGLE_PRECISION_TEST
testTendencyTermsPlaneSinglePrecision.exe
ocn/TendencyTermsTest.cpp
"-n 8;--cpu-bind=cores"
"-n 8;"
single_precision
)

add_omega_test(
TEND_SPHERE_TEST
testTendencyTermsSphere.exe
ocn/TendencyTermsTest.cpp
"-n 8;--cpu-bind=cores"
"-n 8;"
)
target_compile_definitions(
testTendencyTermsSphere.exe
Expand Down Expand Up @@ -350,7 +358,7 @@ add_omega_test(
REDUCTIONS_TEST
testReductions.exe
base/ReductionsTest.cpp
"-n;2;--cpu-bind=cores"
"-n;2;"
)

###################
Expand Down
Loading