Skip to content
Open
Show file tree
Hide file tree
Changes from 12 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
2 changes: 1 addition & 1 deletion .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"build": {
"dockerfile": "Dockerfile",
"args": {
"GEOS_TPL_TAG": "359-1057"
"GEOS_TPL_TAG": "361-1059"
}
},
"runArgs": [
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ jobs:
# frozen at the time the workflow was first triggered, so labels
# added after that first run are invisible to re-runs. The curl
# call below always returns current state, which is required for
# CI re-runs to pick up newly added labels (e.g. "ci: run CUDA
# CI re-runs to pick up newly added labels (e.g. "ci: run device
# builds"). Do not replace with github.event.pull_request.labels.
# The Authorization header is required for private repos/forks;
# GITHUB_TOKEN is provided automatically to every workflow run.
Expand Down
56 changes: 53 additions & 3 deletions .github/workflows/ci_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ jobs:
# - run: sysctl -n machdep.cpu.brand_string


# If the 'ci: run CUDA builds' PR label is found, the cuda jobs run immediately along side linux jobs.
# If the 'ci: run device builds' PR label is found, the CUDA jobs run immediately along side linux jobs.
# Note: CUDA jobs should only be run if PR is ready to merge.
cuda_builds:
name: ${{ matrix.name }}
Expand Down Expand Up @@ -433,10 +433,57 @@ jobs:
NPROC: ${{ matrix.NPROC }}
CTEST_PARALLEL_LEVEL: ${{ matrix.CTEST_PARALLEL_LEVEL || matrix.NPROC }}
RUNS_ON: ${{ matrix.RUNS_ON }}
REQUIRED_LABEL: "ci: run CUDA builds"
REQUIRED_LABEL: "ci: run device builds"
secrets: inherit

# Convenience job - passes when all other jobs have passed (must pass the CUDA jobs).
# If the 'ci: run device builds' PR label is found, the ROCm job runs alongside
# the CPU and CUDA jobs. This is a build-only job, so the runner does not need
# an AMD GPU; the image supplies the amdclang 19/ROCm 6.4.3 toolchain and the
# generated /spack-generated.cmake host-config.
rocm_builds:
name: ${{ matrix.name }}
needs:
- is_not_draft_pull_request
strategy:
fail-fast: false
matrix:
include:
- name: Ubuntu 24.04 - amdclang 19 + ROCm 6.4.3
BUILD_AND_TEST_CLI_ARGS: "--build-exe-only --no-install-schema"
CMAKE_BUILD_TYPE: Release
BUILD_GENERATOR: "--ninja"
DOCKER_REPOSITORY: geosx/ubuntu24.04-amdclang19.0.0-rocm6.4.3
ENABLE_HYPRE_DEVICE: HIP
ENABLE_HYPRE: ON
ENABLE_HYPREDRV: ON
ENABLE_TRILINOS: OFF
GEOS_ENABLE_BOUNDS_CHECK: OFF
RUNS_ON: streak2
NPROC: 8
DOCKER_RUN_ARGS: "--cpus=8 --memory=128g -e ROCM_PATH=/opt/rocm-6.4.3 -e HIP_PATH=/opt/rocm-6.4.3 -v /etc/pki/tls/certs/ca-bundle.crt:/etc/pki/tls/certs/ca-bundle.crt:ro -v /etc/pki/tls/certs/ca-bundle.crt:/certs/ca-bundle.crt:ro"
HOST_CONFIG: /spack-generated.cmake

uses: ./.github/workflows/build_and_test.yml
with:
BUILD_AND_TEST_CLI_ARGS: ${{ matrix.BUILD_AND_TEST_CLI_ARGS }}
CMAKE_BUILD_TYPE: ${{ matrix.CMAKE_BUILD_TYPE }}
BUILD_GENERATOR: ${{ matrix.BUILD_GENERATOR }}
DOCKER_IMAGE_TAG: ${{ needs.is_not_draft_pull_request.outputs.DOCKER_IMAGE_TAG }}
DOCKER_REPOSITORY: ${{ matrix.DOCKER_REPOSITORY }}
DOCKER_RUN_ARGS: ${{ matrix.DOCKER_RUN_ARGS }}
ENABLE_HYPRE_DEVICE: ${{ matrix.ENABLE_HYPRE_DEVICE }}
ENABLE_HYPRE: ${{ matrix.ENABLE_HYPRE }}
ENABLE_HYPREDRV: ${{ matrix.ENABLE_HYPREDRV }}
ENABLE_TRILINOS: ${{ matrix.ENABLE_TRILINOS }}
GEOS_ENABLE_BOUNDS_CHECK: ${{ matrix.GEOS_ENABLE_BOUNDS_CHECK }}
HOST_CONFIG: ${{ matrix.HOST_CONFIG }}
NPROC: ${{ matrix.NPROC }}
CTEST_PARALLEL_LEVEL: ${{ matrix.CTEST_PARALLEL_LEVEL || matrix.NPROC }}
RUNS_ON: ${{ matrix.RUNS_ON }}
REQUIRED_LABEL: "ci: run device builds"
secrets: inherit

# Convenience job - passes when all other jobs have passed (including the GPU jobs).
check_that_all_jobs_succeeded:
runs-on: ubuntu-22.04
needs:
Expand All @@ -446,6 +493,7 @@ jobs:
- check_code_rules
- cpu_builds
- cuda_builds
- rocm_builds
- run_integrated_tests
if: ${{ always() }}
steps:
Expand All @@ -455,12 +503,14 @@ jobs:
echo "check_code_style_and_documentation: ${{needs.check_code_style_and_documentation.result}}"
echo "cpu_builds: ${{needs.cpu_builds.result}}"
echo "cuda_builds: ${{needs.cuda_builds.result}}"
echo "rocm_builds: ${{needs.rocm_builds.result}}"
echo "run_integrated_tests: ${{needs.run_integrated_tests.result}} "
${{
needs.if_not_unassigned_pull_request.result == 'success' &&
needs.are_submodules_in_sync.result == 'success' &&
needs.check_code_style_and_documentation.result == 'success' &&
needs.cpu_builds.result == 'success' &&
needs.cuda_builds.result == 'success' &&
needs.rocm_builds.result == 'success' &&
needs.run_integrated_tests.result == 'success'
}}
14 changes: 13 additions & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ cmake_policy(SET CMP0056 NEW) # use CMAKE_EXE_LINKER_FLAGS in try_compile() in a
################################
project( geosx LANGUAGES C CXX )
include(GNUInstallDirs)
set( BLT_CXX_STD "c++17" CACHE STRING "Version of C++ standard" )
set( BLT_CXX_STD "c++20" CACHE STRING "Version of C++ standard" )
if(CMAKE_BUILD_TYPE EQUAL "Debug")
set( ENABLE_WARNINGS_AS_ERRORS "OFF" CACHE PATH "")
else()
Expand Down Expand Up @@ -50,6 +50,18 @@ option( ENABLE_BENCHMARKS "Enables benchmarks" ON )
include( cmake/blt/SetupBLT.cmake )
set( BLT_SOURCE_DIR ${PROJECT_SOURCE_DIR}/cmake/blt/ )

# BLT's bundled GoogleTest currently uses an implicit char8_t-to-char32_t
# conversion that Clang 22 diagnoses as -Wcharacter-conversion. Keep GEOS'
# warnings-as-errors policy intact while limiting the compatibility flag to
# the third-party test target.
if( CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 22 )
foreach( _geos_gtest_target gtest gtest_main gmock gmock_main )
if( TARGET ${_geos_gtest_target} )
target_compile_options( ${_geos_gtest_target} PRIVATE -Wno-error=character-conversion )
endif()
endforeach()
endif()

# use, i.e. don't skip the full RPATH for the build tree
set( CMAKE_SKIP_BUILD_RPATH FALSE )

Expand Down
27 changes: 20 additions & 7 deletions src/cmake/GeosxOptions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,9 @@ option( GEOS_ENABLE_WAVEPROPAGATION "Enables wave propagation physics package" O
#message( "SPHINX_FOUND = ${SPHINX_FOUND}" )
#message( "SPHINX_EXECUTABLE = ${SPHINX_EXECUTABLE}" )

if( NOT BLT_CXX_STD STREQUAL c++17 )
MESSAGE( FATAL_ERROR "c++17 is NOT enabled. GEOSX requires c++17" )
endif( NOT BLT_CXX_STD STREQUAL c++17 )
if( NOT BLT_CXX_STD STREQUAL c++20 )
MESSAGE( FATAL_ERROR "c++20 is NOT enabled. GEOSX requires c++20" )
Comment thread
victorapm marked this conversation as resolved.
Outdated
endif( NOT BLT_CXX_STD STREQUAL c++20 )

message( "CMAKE_CXX_COMPILER_ID = ${CMAKE_CXX_COMPILER_ID}" )

Expand All @@ -162,6 +162,13 @@ blt_append_custom_compiler_flag( FLAGS_VAR CMAKE_CXX_FLAGS
CLANG "-Wpedantic -pedantic-errors -Wshadow -Wfloat-equal -Wno-cast-align -Wcast-qual"
)

if( ENABLE_HIP )
# amdclang compiles C++ sources through the HIP driver. GEOS has existing
# [=] lambdas that implicitly capture this; C++20 diagnoses that pattern.
blt_append_custom_compiler_flag( FLAGS_VAR CMAKE_CXX_FLAGS
CLANG "-Wno-deprecated-this-capture -Wno-unused-parameter -Wno-unused-variable -Wno-unused-lambda-capture -Wno-gpu-maybe-wrong-side" )
endif()

blt_append_custom_compiler_flag( FLAGS_VAR CMAKE_CXX_FLAGS_DEBUG
GNU "-Wno-unused-parameter -Wno-unused-variable"
CLANG "-Wno-unused-parameter -Wno-unused-variable -fstandalone-debug"
Expand All @@ -188,11 +195,17 @@ if (ENABLE_GBENCHMARK)
endif()

if( GEOS_ENABLE_FPE )
check_cxx_compiler_flag( "-ffp-exception-behavior=strict" GEOS_CXX_HAS_FP_EXCEPTION_BEHAVIOR_STRICT)
if( GEOS_CXX_HAS_FP_EXCEPTION_BEHAVIOR_STRICT )
blt_append_custom_compiler_flag( FLAGS_VAR CMAKE_CXX_FLAGS CLANG "-ffp-exception-behavior=strict" )
# amdclang compiles HIP sources through the CXX driver. The host-only
# floating-point exception flag is rejected for the device compilation.
if( ENABLE_HIP )
message( STATUS "GEOS_ENABLE_FPE is ON, but HIP builds do not support -ffp-exception-behavior=strict; skipping the flag." )
else()
message( WARNING "GEOS_ENABLE_FPE is ON, but ${CMAKE_CXX_COMPILER_ID} does not support -ffp-exception-behavior=strict." )
check_cxx_compiler_flag( "-ffp-exception-behavior=strict" GEOS_CXX_HAS_FP_EXCEPTION_BEHAVIOR_STRICT)
if( GEOS_CXX_HAS_FP_EXCEPTION_BEHAVIOR_STRICT )
blt_append_custom_compiler_flag( FLAGS_VAR CMAKE_CXX_FLAGS CLANG "-ffp-exception-behavior=strict" )
else()
message( WARNING "GEOS_ENABLE_FPE is ON, but ${CMAKE_CXX_COMPILER_ID} does not support -ffp-exception-behavior=strict." )
endif()
endif()
endif()

Expand Down
2 changes: 1 addition & 1 deletion src/coreComponents/LvArray
2 changes: 1 addition & 1 deletion src/coreComponents/common/LifoStorageCuda.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ class LifoStorageCuda : public LifoStorageCommon< T, INDEX_TYPE >
{
LIFO_MARK_FUNCTION;
// The copy to host will only start when the data is copied on device buffer
baseLifo::m_hostDeque.getStream().wait_for( const_cast< camp::resources::Event * >( &m_pushToDeviceEvents[id] ) );
baseLifo::m_hostDeque.getStream().wait_for( m_pushToDeviceEvents[id] );
baseLifo::m_hostDeque.emplaceFrontFromBack( m_deviceDeque );

if( baseLifo::m_maxNumberOfBuffers - id > (int)(m_deviceDeque.capacity() + baseLifo::m_hostDeque.capacity()) )
Expand Down
21 changes: 6 additions & 15 deletions src/coreComponents/common/format/Format.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@
#include <type_traits>
#include <optional>

#if __cplusplus < 202002L
#define GEOS_USE_FMT
#endif

#ifdef GEOS_USE_FMT
#ifndef FMT_HEADER_ONLY
Expand All @@ -33,9 +31,6 @@
#include "../include/fmt/ranges.h"
#include "../include/fmt/xchar.h"
#define GEOS_FMT_NS fmt
#else // use C++20's <format>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused here. Since we are not enforcing C++20, shouldn't we always use <format>?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi Dick, this branch is enforcing C++20, but {fmt} remains intentional because GEOS uses fmt::runtime, fmt::join, fmt::is_formattable and custom fmt::formatter. Also, <format> is not uniformly supported by the CUDA/HIP backends. Maybe we can address this on a separate PR

#include <format>
#define GEOS_FMT_NS std
#endif

#ifdef GEOS_USE_FMT
Expand Down Expand Up @@ -80,6 +75,12 @@ struct fmt::formatter< T, std::enable_if_t< std::is_enum< T >::value > >
*/
#define GEOS_FMT( msg, ... ) GEOS_FMT_NS::format( msg, __VA_ARGS__ )

/**
* @brief Interpolate arguments into a run-time format string.
* @param msg the message format string, evaluated at run time
*/
#define GEOS_FMT_RUNTIME( msg, ... ) GEOS_FMT_NS::format( GEOS_FMT_NS::runtime( msg ), __VA_ARGS__ )

/**
* @brief Interpolate arguments into a message format string and write into an output iterator.
* @param iter the output iterator to write to
Expand Down Expand Up @@ -140,18 +141,8 @@ constexpr auto GEOS_FMT_NS::detail::has_const_formatter_impl< GEOS_FMT_NS::forma
/**
* Evaluates at compile time if a fmt::formatter exists for a given type
*/
#if __cplusplus < 202002L
// fmt 11.2: has_formatter<T, Context>() no longer works. The second parameter
// is now Char (not format_context), and the type-trait form is deprecated.
template< class T >
static constexpr bool has_formatter_v = fmt::is_formattable< fmt::remove_cvref_t< T > >::value;
#else
template< typename T >
concept has_formatter_v = requires ( T& v, std::format_context ctx )
{
std::formatter< std::remove_cvref_t< T > >().format( v, ctx );
};
#endif

namespace geos::format
{
Expand Down
4 changes: 2 additions & 2 deletions src/coreComponents/common/format/table/TableData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ TableData2D::TableDataHolder TableData2D::buildTableData( string_view targetUnit

for( auto const & columnValue : m_columnValues )
{
tableData1D.headerNames.push_back( GEOS_FMT( columnFmt, columnValue ) );
tableData1D.headerNames.push_back( GEOS_FMT_RUNTIME( columnFmt, columnValue ) );
}

for( auto const & error : *m_errors )
Expand All @@ -161,7 +161,7 @@ TableData2D::TableDataHolder TableData2D::buildTableData( string_view targetUnit
{
stdVector< TableData::CellData > currentRowValues;
currentRowValues.reserve( rowMap.size() );
currentRowValues.push_back( {CellType::Value, GEOS_FMT( rowFmt, rowValue )} );
currentRowValues.push_back( {CellType::Value, GEOS_FMT_RUNTIME( rowFmt, rowValue )} );

std::set< real64 >::const_iterator columnIt = m_columnValues.begin();
for( auto const & [columnValue, cellValue] : rowMap )
Expand Down
3 changes: 0 additions & 3 deletions src/coreComponents/common/logger/Logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@
{ \
if( COND ) \
{ \
GEOS_UNUSED_VAR( GEOS_DETAIL_FIRST_ARG( __VA_ARGS__ ) ); \
constexpr char const * formatString = "***** ERROR\n" \
"***** LOCATION" LOCATION "\n" \
"***** BLOCK: [%u, %u, %u]\n" \
Expand Down Expand Up @@ -278,8 +277,6 @@
{ \
if( COND ) \
{ \
::geos::internal::DeviceNullStream __geosNullStream; \
__geosNullStream << MSG; \
static char const formatString[] = "***** ERROR\n" \
"***** LOCATION" LOCATION "\n" \
"***** BLOCK: [%u, %u, %u]\n" \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ TEST( FixedSizeDequeTest, emplace_and_pop_front_cuda )
for( int i = 0; i < elemCnt; i++ )
array[i] = i + j *maxArray;
camp::resources::Event e = deque.emplace_front( array.toSliceConst() );
stream.wait_for( &e );
stream.wait_for( e );
if( j+1 < maxArray )
{
EXPECT_EQ( false, deque.empty());
Expand Down
2 changes: 1 addition & 1 deletion src/coreComponents/common/unitTests/testMpiWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ string reductionTestFailureMsg( PairTestCase< FIRST, SECOND > const & testCase,
int rankId,
string_view opName )
{
return GEOS_FMT( "Test case '{}', rank {}:\n Error in {} reduction, incorrect first value in pair ({}, {}}).",
return GEOS_FMT( "Test case '{}', rank {}:\n Error in {} reduction, incorrect first value in pair ({}, {}).",
testCase.testName, rankId, opName, pair.first, pair.second );
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,8 @@ void BlackOilFluidBase::checkTablesParameters( real64 const pressure,
m_formationVolFactorTables[iph]->checkCoord( pressure, 0 );
} catch( SimulationError const & ex )
{
string const exceptionMsg = GEOS_FMT( errorMsg, getCatalogName(), getDataContext(),
"formation volume factor", iph );
string const exceptionMsg = GEOS_FMT_RUNTIME( errorMsg, getCatalogName(), getDataContext(),
"formation volume factor", iph );
ErrorLogger::global().modifyCurrentExceptionMessage()
.addToMsg( exceptionMsg )
.addContextInfo( getDataContext().getContextInfo().setPriority( 2 ) );
Expand All @@ -268,8 +268,8 @@ void BlackOilFluidBase::checkTablesParameters( real64 const pressure,
m_viscosityTables[iph]->checkCoord( pressure, 0 );
} catch( SimulationError const & ex )
{
string const exceptionMsg = GEOS_FMT( errorMsg, getCatalogName(), getDataContext(),
"viscosity", iph );
string const exceptionMsg = GEOS_FMT_RUNTIME( errorMsg, getCatalogName(), getDataContext(),
"viscosity", iph );

ErrorLogger::global().modifyCurrentExceptionMessage()
.addToMsg( exceptionMsg )
Expand Down
Loading
Loading