From 9f50193d6205379438b1e26da38a5b778d7873b2 Mon Sep 17 00:00:00 2001 From: dkachuma Date: Tue, 7 Apr 2026 12:31:50 -0500 Subject: [PATCH 1/7] Fix MPI sync --- .../fluidFlow/SourceFluxStatistics.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp index 26f734efd64..450b0ea6df9 100644 --- a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp +++ b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp @@ -139,6 +139,14 @@ void SourceFluxStatsAggregator::registerDataOnMesh( Group & meshBodies ) } ); } } ); + + if( m_writeCSV > 0 && MpiWrapper::commRank() == 0 ) + { + std::ofstream outputFile( m_csvFilename ); + TableCSVFormatter const tableStatFormatter( m_csvLayout ); + outputFile << tableStatFormatter.headerToString(); + outputFile.close(); + } } void SourceFluxStatsAggregator::gatherStatsForLog( bool logLevelActive, @@ -215,9 +223,9 @@ void SourceFluxStatsAggregator::outputStatsToCSV( TableData & csvData ) { if( m_writeCSV > 0 && MpiWrapper::commRank() == 0 ) { - std::ofstream outputFile( m_csvFilename ); + std::ofstream outputFile( m_csvFilename, std::ios::app ); TableCSVFormatter const tableStatFormatter( m_csvLayout ); - outputFile << tableStatFormatter.toString( csvData ); + outputFile << tableStatFormatter.dataToString( csvData ); outputFile.close(); csvData.clear(); } @@ -241,19 +249,20 @@ bool SourceFluxStatsAggregator::execute( real64 const GEOS_UNUSED_PARAM( time_n { TableData logData; TableData csvData; - meshLevelStats.stats() = StatData(); + meshLevelStats.stats().reset(); forAllFluxStatsWrappers( meshLevel, [&] ( MeshLevel &, WrappedStats & fluxStats ) { - fluxStats.stats() = StatData(); + fluxStats.stats().reset(); forAllRegionStatsWrappers( meshLevel, fluxStats.getFluxName(), [&] ( ElementRegionBase & region, WrappedStats & regionStats ) { - regionStats.stats() = StatData(); + regionStats.stats().reset(); forAllSubRegionStatsWrappers( region, regionStats.getFluxName(), [&] ( ElementSubRegionBase &, WrappedStats & subRegionStats ) { + subRegionStats.stats().reset(); subRegionStats.finalizePeriod(); regionStats.stats().combine( subRegionStats.stats() ); } ); From 07de379efeb6da9ead88709032d2fd22df1e394d Mon Sep 17 00:00:00 2001 From: dkachuma Date: Wed, 13 May 2026 13:42:48 -0500 Subject: [PATCH 2/7] Sync times --- .../fluidFlow/SourceFluxStatistics.cpp | 15 ++++++++++----- .../fluidFlow/SourceFluxStatistics.hpp | 12 ++++++++++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp index 450b0ea6df9..f5e918d2e02 100644 --- a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp +++ b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp @@ -231,7 +231,7 @@ void SourceFluxStatsAggregator::outputStatsToCSV( TableData & csvData ) } } -bool SourceFluxStatsAggregator::execute( real64 const GEOS_UNUSED_PARAM( time_n ), +bool SourceFluxStatsAggregator::execute( real64 const GEOS_UNUSED_PARAM ( time_n ), real64 const GEOS_UNUSED_PARAM( dt ), integer const GEOS_UNUSED_PARAM( cycleNumber ), integer const GEOS_UNUSED_PARAM( eventCounter ), @@ -264,16 +264,16 @@ bool SourceFluxStatsAggregator::execute( real64 const GEOS_UNUSED_PARAM( time_n { subRegionStats.stats().reset(); subRegionStats.finalizePeriod(); - regionStats.stats().combine( subRegionStats.stats() ); + regionStats.combine( subRegionStats ); } ); - fluxStats.stats().combine( regionStats.stats() ); + fluxStats.combine( regionStats ); gatherStatsForLog( regionsStatsOn, fluxStats.getFluxName(), region.getName(), logData, regionStats ); gatherStatsForCSV( fluxStats.getFluxName(), region.getName(), csvData, regionStats ); } ); - meshLevelStats.stats().combine( fluxStats.stats() ); + meshLevelStats.combine( fluxStats ); gatherStatsForLog( fluxesStatsOn, fluxStats.getFluxName(), allRegionsStr, logData, fluxStats ); @@ -379,7 +379,7 @@ void SourceFluxStatsAggregator::WrappedStats::finalizePeriod() // produce the period stats of this rank m_stats.m_elementCount = m_periodStats.m_elementCount; - m_statsPeriodStart = m_periodStats.m_periodStart; + m_statsPeriodStart = MpiWrapper::max( m_periodStats.m_periodStart ); m_statsPeriodDT = m_periodStats.m_timeStepDeltaTime + m_periodStats.m_periodPendingDeltaTime; real64 const timeDivisor = m_statsPeriodDT > 0.0 ? 1.0 / m_statsPeriodDT : 0.0; @@ -396,6 +396,11 @@ void SourceFluxStatsAggregator::WrappedStats::finalizePeriod() // start a new timestep m_periodStats.reset(); } +void SourceFluxStatsAggregator::WrappedStats::combine( WrappedStats const & other ) +{ + stats().combine( other.stats() ); + m_statsPeriodStart = LvArray::math::max( m_statsPeriodStart, other.m_statsPeriodStart ); +} void SourceFluxStatsAggregator::WrappedStats::PeriodStats::allocate( integer phaseCount ) { if( m_timeStepMass.size() < phaseCount ) diff --git a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.hpp b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.hpp index b777cd3ba07..5b015e22232 100644 --- a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.hpp +++ b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.hpp @@ -112,6 +112,14 @@ class SourceFluxStatsAggregator final : public FieldStatisticsBase< FlowSolverBa */ void finalizePeriod(); + /** + * @brief Aggregate the statistics of the instance with those of another one. + * @details Combines the statistics from the other object into this one and also advances the period + * start if the other object has a later time recorded. + * @param other the other WrappedStats object. + */ + void combine( WrappedStats const & other ); + /** * @return the reference to the wrapped stats data collected over the last period (one timestep or more), computed by finalizePeriod() */ @@ -151,7 +159,7 @@ class SourceFluxStatsAggregator final : public FieldStatisticsBase< FlowSolverBa /// stats data collected over the last period (one timestep or more), computed by finalizePeriod() StatData m_stats; /// the start time of the wrapped stats period (in s) - real64 m_statsPeriodStart; + real64 m_statsPeriodStart{-LvArray::NumericLimits< real64 >::max}; /// the duration of the wrapped stats period (in s) real64 m_statsPeriodDT; @@ -169,7 +177,7 @@ class SourceFluxStatsAggregator final : public FieldStatisticsBase< FlowSolverBa /// time that the current timestep is simulating. real64 m_timeStepDeltaTime = 0.0; /// start time of the current period. - real64 m_periodStart = 0.0; + real64 m_periodStart = -LvArray::NumericLimits< real64 >::max; /// delta time from all previous time-step of the current period. real64 m_periodPendingDeltaTime = 0.0; /// number of cell elements targeted by this instance From b9f8ba43c07cfddf5cd6aefc57244a8731d5496e Mon Sep 17 00:00:00 2001 From: dkachuma Date: Wed, 13 May 2026 13:47:48 -0500 Subject: [PATCH 3/7] Trivial change --- .../physicsSolvers/fluidFlow/SourceFluxStatistics.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp index f5e918d2e02..ef69d0ae326 100644 --- a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp +++ b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp @@ -372,6 +372,7 @@ void SourceFluxStatsAggregator::WrappedStats::gatherTimeStepStats( real64 const m_periodStats.m_timeStepMass = producedMass; } } + void SourceFluxStatsAggregator::WrappedStats::finalizePeriod() { // init phase data memory allocation if needed From e68749d6435e209d2e35618b4ac96d0b8f6effd2 Mon Sep 17 00:00:00 2001 From: dkachuma Date: Wed, 13 May 2026 13:47:56 -0500 Subject: [PATCH 4/7] Trivial change --- .../physicsSolvers/fluidFlow/SourceFluxStatistics.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp index ef69d0ae326..f5e918d2e02 100644 --- a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp +++ b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp @@ -372,7 +372,6 @@ void SourceFluxStatsAggregator::WrappedStats::gatherTimeStepStats( real64 const m_periodStats.m_timeStepMass = producedMass; } } - void SourceFluxStatsAggregator::WrappedStats::finalizePeriod() { // init phase data memory allocation if needed From b971c1bedeb3faaee2c3055df7aeba1c1f44c5a3 Mon Sep 17 00:00:00 2001 From: dkachuma Date: Wed, 20 May 2026 13:32:07 -0500 Subject: [PATCH 5/7] Fix elemet count and add unit test --- .../fluidFlow/CompositionalMultiphaseBase.cpp | 7 +++++-- .../physicsSolvers/fluidFlow/SinglePhaseBase.cpp | 11 ++++++++--- .../fluidFlow/SourceFluxStatistics.cpp | 2 +- .../fluidFlow/unitTests/CMakeLists.txt | 7 +++++++ .../fluidFlow/unitTests/testFlowStatistics.cpp | 16 ++++++++++------ 5 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/coreComponents/physicsSolvers/fluidFlow/CompositionalMultiphaseBase.cpp b/src/coreComponents/physicsSolvers/fluidFlow/CompositionalMultiphaseBase.cpp index 29c200f034a..aec610cfa41 100644 --- a/src/coreComponents/physicsSolvers/fluidFlow/CompositionalMultiphaseBase.cpp +++ b/src/coreComponents/physicsSolvers/fluidFlow/CompositionalMultiphaseBase.cpp @@ -1877,6 +1877,7 @@ void CompositionalMultiphaseBase::applySourceFluxBC( real64 const time, localIndex const rankOffset = dofManager.rankOffset(); RAJA::ReduceSum< parallelDeviceReduce, real64 > massProd( 0.0 ); + RAJA::ReduceSum< parallelDeviceReduce, localIndex > elementCount( 0 ); // note that the dofArray will not be used after this step (simpler to use dofNumber instead) FieldSpecificationImpl::computeRhsContribution< FieldSpecificationAdd, @@ -1913,7 +1914,8 @@ void CompositionalMultiphaseBase::applySourceFluxBC( real64 const time, dofNumber, rhsContributionArrayView, localRhs, - massProd] GEOS_HOST_DEVICE ( localIndex const a ) + massProd, + elementCount] GEOS_HOST_DEVICE ( localIndex const a ) { // we need to filter out ghosts here, because targetSet may contain them localIndex const ei = targetSet[a]; @@ -1924,6 +1926,7 @@ void CompositionalMultiphaseBase::applySourceFluxBC( real64 const time, real64 const rhsValue = rhsContributionArrayView[a] / sizeScalingFactor; // scale the contribution by the sizeScalingFactor here! massProd += rhsValue; + elementCount += 1; if( useTotalMassEquation > 0 ) { // for all "fluid components", we add the value to the total mass balance equation @@ -1948,7 +1951,7 @@ void CompositionalMultiphaseBase::applySourceFluxBC( real64 const time, // set the new sub-region statistics for this timestep array1d< real64 > massProdArr{ m_numComponents }; massProdArr[fluidComponentId] = massProd.get(); - wrapper.gatherTimeStepStats( time, dt, massProdArr.toViewConst(), targetSet.size() ); + wrapper.gatherTimeStepStats( time, dt, massProdArr.toViewConst(), elementCount.get() ); } ); } ); } ); diff --git a/src/coreComponents/physicsSolvers/fluidFlow/SinglePhaseBase.cpp b/src/coreComponents/physicsSolvers/fluidFlow/SinglePhaseBase.cpp index 562cfae6df4..5b04db3c016 100644 --- a/src/coreComponents/physicsSolvers/fluidFlow/SinglePhaseBase.cpp +++ b/src/coreComponents/physicsSolvers/fluidFlow/SinglePhaseBase.cpp @@ -1055,6 +1055,7 @@ void SinglePhaseBase::applySourceFluxBC( real64 const time_n, localIndex const rankOffset = dofManager.rankOffset(); RAJA::ReduceSum< parallelDeviceReduce, real64 > massProd( 0.0 ); + RAJA::ReduceSum< parallelDeviceReduce, localIndex > elementCount( 0 ); // note that the dofArray will not be used after this step (simpler to use dofNumber instead) FieldSpecificationImpl::computeRhsContribution< FieldSpecificationAdd, @@ -1096,7 +1097,8 @@ void SinglePhaseBase::applySourceFluxBC( real64 const time_n, rhsContributionArrayView, localRhs, localMatrix, - massProd] GEOS_HOST_DEVICE ( localIndex const a ) + massProd, + elementCount] GEOS_HOST_DEVICE ( localIndex const a ) { // we need to filter out ghosts here, because targetSet may contain them localIndex const ei = targetSet[a]; @@ -1111,6 +1113,7 @@ void SinglePhaseBase::applySourceFluxBC( real64 const time_n, real64 const rhsValue = rhsContributionArrayView[a] / sizeScalingFactor; // scale the contribution by the sizeScalingFactor here! localRhs[massRowIndex] += rhsValue; massProd += rhsValue; + elementCount += 1; //add the value to the energy balance equation if the flux is positive (i.e., it's a producer) if( rhsContributionArrayView[a] > 0.0 ) { @@ -1138,7 +1141,8 @@ void SinglePhaseBase::applySourceFluxBC( real64 const time_n, dofNumber, rhsContributionArrayView, localRhs, - massProd] GEOS_HOST_DEVICE ( localIndex const a ) + massProd, + elementCount] GEOS_HOST_DEVICE ( localIndex const a ) { // we need to filter out ghosts here, because targetSet may contain them localIndex const ei = targetSet[a]; @@ -1152,6 +1156,7 @@ void SinglePhaseBase::applySourceFluxBC( real64 const time_n, real64 const rhsValue = rhsContributionArrayView[a] / sizeScalingFactor; localRhs[rowIndex] += rhsValue; massProd += rhsValue; + elementCount += 1; } ); } @@ -1161,7 +1166,7 @@ void SinglePhaseBase::applySourceFluxBC( real64 const time_n, // set the new sub-region statistics for this timestep array1d< real64 > massProdArr{ 1 }; massProdArr[0] = massProd.get(); - wrapper.gatherTimeStepStats( time_n, dt, massProdArr.toViewConst(), targetSet.size() ); + wrapper.gatherTimeStepStats( time_n, dt, massProdArr.toViewConst(), elementCount.get() ); } ); } ); } ); diff --git a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp index f5e918d2e02..a4bf237b134 100644 --- a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp +++ b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp @@ -231,7 +231,7 @@ void SourceFluxStatsAggregator::outputStatsToCSV( TableData & csvData ) } } -bool SourceFluxStatsAggregator::execute( real64 const GEOS_UNUSED_PARAM ( time_n ), +bool SourceFluxStatsAggregator::execute( real64 const GEOS_UNUSED_PARAM( time_n ), real64 const GEOS_UNUSED_PARAM( dt ), integer const GEOS_UNUSED_PARAM( cycleNumber ), integer const GEOS_UNUSED_PARAM( eventCounter ), diff --git a/src/coreComponents/physicsSolvers/fluidFlow/unitTests/CMakeLists.txt b/src/coreComponents/physicsSolvers/fluidFlow/unitTests/CMakeLists.txt index 77d628e7ae2..f59e71ec23b 100644 --- a/src/coreComponents/physicsSolvers/fluidFlow/unitTests/CMakeLists.txt +++ b/src/coreComponents/physicsSolvers/fluidFlow/unitTests/CMakeLists.txt @@ -17,4 +17,11 @@ foreach( test_source ${fluidFlow_gtest_tests} ) FOLDER UnitTests ) geos_add_test( NAME ${test_name} COMMAND ${test_name} ) + + if( ENABLE_MPI AND NOT ENABLE_CUDA AND NOT ENABLE_HIP ) + set( nranks 2 ) + geos_add_test( NAME ${test_name}_mpi + COMMAND ${test_name} -x ${nranks} + NUM_MPI_TASKS ${nranks} ) + endif() endforeach() diff --git a/src/coreComponents/physicsSolvers/fluidFlow/unitTests/testFlowStatistics.cpp b/src/coreComponents/physicsSolvers/fluidFlow/unitTests/testFlowStatistics.cpp index 356661e434e..b8c1207893b 100644 --- a/src/coreComponents/physicsSolvers/fluidFlow/unitTests/testFlowStatistics.cpp +++ b/src/coreComponents/physicsSolvers/fluidFlow/unitTests/testFlowStatistics.cpp @@ -19,6 +19,7 @@ #include "mainInterface/GeosxState.hpp" #include "physicsSolvers/fluidFlow/SourceFluxStatistics.hpp" #include "physicsSolvers/fluidFlow/SinglePhaseStatistics.hpp" +#include "common/MpiWrapper.hpp" #include @@ -154,14 +155,17 @@ class FlowStatisticsTest : public ::testing::Test void writeTableFiles( stdMap< string, string > const & files ) { - for( auto const & [fileName, content] : files ) + if( MpiWrapper::commRank() == 0 ) { - std::ofstream os( fileName ); - ASSERT_TRUE( os.is_open() ); - os << content; - os.close(); + for( auto const & [fileName, content] : files ) + { + std::ofstream os( fileName ); + ASSERT_TRUE( os.is_open() ); + os << content; + os.close(); - m_tableFileNames.push_back( fileName ); + m_tableFileNames.push_back( fileName ); + } } } From 4f0b388034701ed289ae21dbee1d4b07262f079a Mon Sep 17 00:00:00 2001 From: Dickson Kachuma <81433670+dkachuma@users.noreply.github.com> Date: Thu, 21 May 2026 01:28:18 -0500 Subject: [PATCH 6/7] Add barrier call in testFlowStatistics setup --- .../physicsSolvers/fluidFlow/unitTests/testFlowStatistics.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreComponents/physicsSolvers/fluidFlow/unitTests/testFlowStatistics.cpp b/src/coreComponents/physicsSolvers/fluidFlow/unitTests/testFlowStatistics.cpp index b8c1207893b..61d85956613 100644 --- a/src/coreComponents/physicsSolvers/fluidFlow/unitTests/testFlowStatistics.cpp +++ b/src/coreComponents/physicsSolvers/fluidFlow/unitTests/testFlowStatistics.cpp @@ -167,6 +167,7 @@ class FlowStatisticsTest : public ::testing::Test m_tableFileNames.push_back( fileName ); } } + MpiWrapper::barrier(); } void TearDown() override From a7f089401ea254e65265e4f1f9c9d07bb82b150d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 May 2026 19:44:40 +0000 Subject: [PATCH 7/7] Add comments explaining max() usage for period start time in SourceFluxStatistics Agent-Logs-Url: https://github.com/GEOS-DEV/GEOS/sessions/bcea92d2-4f51-434e-9036-5f81b1953661 Co-authored-by: dkachuma <81433670+dkachuma@users.noreply.github.com> --- .../physicsSolvers/fluidFlow/SourceFluxStatistics.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp index a4bf237b134..5ae6cf4152e 100644 --- a/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp +++ b/src/coreComponents/physicsSolvers/fluidFlow/SourceFluxStatistics.cpp @@ -379,6 +379,8 @@ void SourceFluxStatsAggregator::WrappedStats::finalizePeriod() // produce the period stats of this rank m_stats.m_elementCount = m_periodStats.m_elementCount; + // MPI ranks without cells in the flux region may bypass gatherTimeStepStats() entirely, + // leaving their m_periodStart behind. Take the max so all ranks agree on the most advanced time. m_statsPeriodStart = MpiWrapper::max( m_periodStats.m_periodStart ); m_statsPeriodDT = m_periodStats.m_timeStepDeltaTime + m_periodStats.m_periodPendingDeltaTime; @@ -399,6 +401,8 @@ void SourceFluxStatsAggregator::WrappedStats::finalizePeriod() void SourceFluxStatsAggregator::WrappedStats::combine( WrappedStats const & other ) { stats().combine( other.stats() ); + // Some fluxes may lag behind (their ranks may have skipped gatherTimeStepStats()), + // so take the most advanced period start time across all combined stats. m_statsPeriodStart = LvArray::math::max( m_statsPeriodStart, other.m_statsPeriodStart ); } void SourceFluxStatsAggregator::WrappedStats::PeriodStats::allocate( integer phaseCount )