-
Notifications
You must be signed in to change notification settings - Fork 101
Fix source statistics #4056
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.
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
base: develop
Are you sure you want to change the base?
Fix source statistics #4056
Changes from all commits
9f50193
e5272e7
07de379
c21d3b0
b9f8ba4
e68749d
7de86dd
88f8da6
b971c1b
4f0b388
1b058d9
a7f0894
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,30 +249,31 @@ 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() ); | ||
| 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 ); | ||
|
|
@@ -370,7 +379,9 @@ 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; | ||
| // 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; | ||
|
|
||
| real64 const timeDivisor = m_statsPeriodDT > 0.0 ? 1.0 / m_statsPeriodDT : 0.0; | ||
|
|
@@ -387,6 +398,13 @@ void SourceFluxStatsAggregator::WrappedStats::finalizePeriod() | |
| // start a new timestep | ||
| m_periodStats.reset(); | ||
| } | ||
| 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 ); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't it be
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some of the processes lag behind because the time doesn't get advanced. So we need to pick the most advanced time.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I just would add a short comment to explain this choice. |
||
| } | ||
| void SourceFluxStatsAggregator::WrappedStats::PeriodStats::allocate( integer phaseCount ) | ||
| { | ||
| if( m_timeStepMass.size() < phaseCount ) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can they be different over ranks?
Isn't
SourceFluxStatistics::gatherTimeStepStats()called synchonously?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for mpi ranks without cells in the region, there is a bypass at CompositionalMultiphaseBase.cpp:1813-1816 - haven't checked single phase, but probably the same thing. This means some of the processes don't get to call
SourceFluxStatistics::gatherTimeStepStats()and lag behind. Hence the need for a max.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
Same thing, I would add a short comment to explain why this
max()call.