Skip to content

Conversation

arng40
Copy link
Contributor

@arng40 arng40 commented Oct 13, 2025

Add GEOS_WARNING for the writeStatistics attribute in case of misappropriate values.

Example with hydrafratureSinglePhase2d.xml

 <Hydrofracture
      name="hydrofracture"
      writeStatistics="all"
      solidSolverName="lagsolve"
      flowSolverName="SinglePhaseFlow"
      [...]
    </Hydrofracture>

    <SolidMechanicsLagrangianFEM
      name="lagsolve"
      writeStatistics="iteration"
      [...]/>

    <SinglePhaseFVM
      name="SinglePhaseFlow"
      writeStatistics="iteration"
       [...]/>

We cannt output iteretion information for both lagsolve & SinglePhaseFlow, here the following warning

***** WARNING
***** LOCATION: /path/to/src/coreComponents/physicsSolvers/PhysicsSolverBase.cpp:152
***** Controlling expression (should be false): canOutputIteration
The lagsolve solver cannot output convergence statistics
You can set writeStatistics to `convergence` 
***** WARNING
***** LOCATION: /path/to/src/coreComponents/physicsSolvers/PhysicsSolverBase.cpp:152
***** Controlling expression (should be false): canOutputIteration
The SinglePhaseFlow solver cannot output convergence statistics
You can set writeStatistics to `convergence`

@arng40 arng40 self-assigned this Oct 13, 2025
@arng40 arng40 added the type: feature New feature or request label Oct 13, 2025
@arng40 arng40 marked this pull request as ready for review October 13, 2025 08:49
@arng40 arng40 changed the title Feature/dudes/add stats warning feat: Additonnal context writeStatistics for misappropriate values Oct 13, 2025
Copy link
Contributor

@MelReyCG MelReyCG left a comment

Choose a reason for hiding this comment

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

About the behaviour, when all is requested but only iteration or convergence are allowed, we should have a warning.

Copy link
Contributor

@MelReyCG MelReyCG left a comment

Choose a reason for hiding this comment

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

I suggest to implement the comparison between what can be output and the user request in the cleanup(), because other components can interact with the iteration / convergence output.
Compare *Stats::m_csvOutputOpened and the user input.

@arng40 arng40 requested a review from MelReyCG October 13, 2025 14:44
@paveltomin paveltomin requested review from Copilot and removed request for Guotong-Ren and frankfeifan October 14, 2025 17:00
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds warning messages to inform users when statistics output configuration is inappropriate for the solver type. The PR implements validation logic that checks if solvers can properly output the requested statistics type and provides helpful suggestions to users about the correct configuration.

  • Adds GEOS_WARNING messages for mismatched statistics output configurations
  • Refactors statistics classes to use clearer naming for output request vs. opened state tracking
  • Changes default density model type from linear to exponential in schema

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/coreComponents/schema/schema.xsd Updates default density model type from linear to exponential
src/coreComponents/physicsSolvers/SolverStatistics.hpp Refactors member variables and methods for clearer naming and adds documentation
src/coreComponents/physicsSolvers/SolverStatistics.cpp Updates implementation to use renamed variables and methods
src/coreComponents/physicsSolvers/PhysicsSolverBase.hpp Updates comment documentation for statistics parameter
src/coreComponents/physicsSolvers/PhysicsSolverBase.cpp Adds warning logic for inappropriate statistics configurations and improves documentation
Comments suppressed due to low confidence (1)

src/coreComponents/physicsSolvers/SolverStatistics.cpp:1

  • Using insert() instead of assignment operator will not update existing keys. This changes behavior from the original code which used assignment. Use m_residuals[key] = value to maintain the same behavior.
/*

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

integer const newtonIter );

/**
* @brief Set a residual value given a key ( column ib the CSV )
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'ib' to 'in'.

Suggested change
* @brief Set a residual value given a key ( column ib the CSV )
* @brief Set a residual value given a key ( column in the CSV )

Copilot uses AI. Check for mistakes.


bool wrongIterationCSVOutputRequest = getIterationStats().getCSVOutputRequest() &&
!getIterationStats().getCSVOutputOpened();
bool wrongConvergenceCSVOutputRequest= getConvergenceStats().getCSVOutputRequest() &&
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

Missing space before assignment operator. Should be 'wrongConvergenceCSVOutputRequest = ' for consistency with coding style.

Suggested change
bool wrongConvergenceCSVOutputRequest= getConvergenceStats().getCSVOutputRequest() &&
bool wrongConvergenceCSVOutputRequest = getConvergenceStats().getCSVOutputRequest() &&

Copilot uses AI. Check for mistakes.

@paveltomin paveltomin 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 Oct 15, 2025
@paveltomin paveltomin merged commit 7d027cc into develop Oct 15, 2025
25 of 26 checks passed
@paveltomin paveltomin deleted the feature/dudes/add-stats-warning branch October 15, 2025 18:07
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: feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants