Skip to content

Detach solver statistics save from data save, and always save - #1737

Merged
jwboth merged 3 commits into
developfrom
save-statistics
Aug 7, 2026
Merged

Detach solver statistics save from data save, and always save#1737
jwboth merged 3 commits into
developfrom
save-statistics

Conversation

@jwboth

@jwboth jwboth commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

This PR suggests to detach saving solver statistics from solving data. While the latter only is performed after successful simulation (of a single time step for instationary simulations), the former shall be always performed, including after the simulation. This makes ensures that the solver statistics is up-to-date even in case of a failing simulation. In part related to #1421 and #1471.

Types of changes

What types of changes does this PR introduce to PorePy?
Put an x in the boxes that apply.

  • Minor change (e.g., dependency bumps, broken links).
  • Bugfix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • Testing (contribution related to testing of existing or new functionality).
  • Documentation (contribution related to adding, improving, or fixing documentation).
  • Maintenance (e.g., improve logic and performance, remove obsolete code).
  • Other:

Checklist

Put an x in the boxes that apply or explain briefly why the box is not relevant.

  • The documentation is up-to-date.
  • Static typing is included in the update.
  • This PR does not duplicate existing functionality.
  • The update is covered by the test suite (including tests added in the PR).
  • If new skipped tests have been introduced in this PR, pytest was run with the --run-skipped flag.

@Yuriyzabegaev Yuriyzabegaev left a comment

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.

Thumbs up for this change! I find it very logical to split data save and statistics save.

I have 2 concerns:

  1. Can I still disable statistics writing, or is it always on? We might want to disable it for unrelated tests to avoid spamming with statistics files.

  2. Soon on the way is the sequential nonlinear solver, that will call before_nonlinear_* and after_nonlinear_* multiple times per time step. Imagine poromechanics splitting, there will be 2 newton solvers inside. Each might want to have its own statistics, which will conflict with this logic. This is fine by now, but will require rethinking of who is responsible for writing solver statistics. Let's keep this in mind.

Comment thread src/porepy/models/solution_strategy.py Outdated
Comment thread tests/viz/test_solver_statistics_unit.py Outdated
@jwboth

jwboth commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Thumbs up for this change! I find it very logical to split data save and statistics save.

I have 2 concerns:

  1. Can I still disable statistics writing, or is it always on? We might want to disable it for unrelated tests to avoid spamming with statistics files.

As before, if not providing "solver_statistics_file_name" in the model parameters, no statistics file is writtin to file. So in most test, no solver statistics should be written, I presume only in the solver statistics tests and some workflow tests (I did not check the test suite).

  1. Soon on the way is the sequential nonlinear solver, that will call before_nonlinear_* and after_nonlinear_* multiple times per time step. Imagine poromechanics splitting, there will be 2 newton solvers inside. Each might want to have its own statistics, which will conflict with this logic. This is fine by now, but will require rethinking of who is responsible for writing solver statistics. Let's keep this in mind.

Can't wait to use this :) I see how this can make things complicated.

Comments:

  • I do not attempt to find a unique solution here - I would see several design possibilities and believe there is easy solutions. It really depends on what is of interest to log and later retrieve. Is it the inner and/or outer nonlinear iterations? Type of solver used/stage of the solver? And how to deal with repeated solves due to fails as there is the potential for unsymmetrically many entrance into the subsolves if an earlier solve fails.
  • Currently one statistics "unit" is related to one time step. Adding a nested structure would possibly require a different logic.
  • What is relevant for the saving. One can save as often one wants - after saving once, a second save does not mutate the object. Just omitting saving is not good. And the added saves after failure and simulation enable that both each time step will be logged, as well as the case of simulation failure is correctly reported to statistics.

@jwboth
jwboth force-pushed the save-statistics branch 2 times, most recently from 8a5247a to 2029902 Compare August 7, 2026 08:13

@Yuriyzabegaev Yuriyzabegaev left a comment

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.

Looks good! I just removed extra pass to restore how it was before.

@jwboth
jwboth merged commit 6b0fcd6 into develop Aug 7, 2026
5 checks passed
@jwboth
jwboth deleted the save-statistics branch August 7, 2026 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants