Skip to content

fix: CSV output in coupled simulation#4009

Open
bd713 wants to merge 7 commits into
developfrom
fix/bd713/statisticsCSV
Open

fix: CSV output in coupled simulation#4009
bd713 wants to merge 7 commits into
developfrom
fix/bd713/statisticsCSV

Conversation

@bd713

@bd713 bd713 commented Mar 30, 2026

Copy link
Copy Markdown
Contributor
  • Ensure iteration files from individual solvers are dumped when running a coupled simulation.
  • Prevent multiple ranks from writing to the CSV file.

@bd713 bd713 self-assigned this Mar 30, 2026
@bd713 bd713 added the type: bug Something isn't working label Mar 30, 2026
@bd713 bd713 marked this pull request as ready for review June 10, 2026 22:52
@bd713 bd713 added ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Jun 10, 2026
GEOS_LOG_LEVEL_RANK_0( logInfo::TimeStep, GEOS_FMT( "New dt = {}", stepDt ) );

// notify the solver statistics counter that this is a time step cut
getIterationStats().updateTimeStepCut();

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.

Suggested change
getIterationStats().writeIterationStatsToTable();

Is it intentional not to save the iteration stats for the CoupledSolver?

m_convergenceData.addRow( residualsNormCells );

if( !m_CSVOutputOpened )
if( MpiWrapper::commRank() == 0 )

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.

Should it apply to the whole writeConvergenceStatsToTable() method?

Comment on lines +149 to +152
real64 const setupTime = m_setupTime;
real64 const solveTime = m_solveTime;
resetSolverLinearTime();

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.

Should it go before the first if? Maybe a scoped if can help.

Comment on lines 171 to +172
m_logStream << m_iterationCSVFormatter->headerToString( );
m_CSVOutputOpened = true;
m_CSVOutputOpened = true;

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.

A bit of renaming could clarify: m_logStream -> m_csvStream

// so writeIterationStatsToTable() early-returns: no rows expected.
}

TEST( CoupledSolverStatisticsCSVTest, checkCSVOutput )

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.

IIUC, as this is the first unit-test that tests **Poromechanics solvers, should this test file be renamed testPoromechanics.cpp? (something like that)
To be more general, regrouping high-level integration-tests, can give an opportunity to optimize the loading time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants