Skip to content

feat(idaklu): solver statistics, self-scheduled input-set sweeps and polymorphic Newton - #5782

Merged
BradyPlanden merged 13 commits into
mainfrom
a2/idaklu-stats-scheduling
Oct 1, 2026
Merged

BradyPlanden merged 13 commits into
mainfrom
a2/idaklu-stats-scheduling

Conversation

@BradyPlanden

@BradyPlanden BradyPlanden commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Description

IDAKLU already collects SUNDIALS' integrator counters, but only prints them under print_stats. This PR returns them with every solution from the pybammsolvers binding and exposes them as Solution.solver_statistics, a pybamm.SolverStatistics dataclass (steps, linear-solver setups, nonlinear iterations, nonlinear failures, error-test failures). Counts accumulate across t_eval breakpoints, are kept per input set, and are summed when solutions combine (+, stepping, experiments), so solver work can be asserted in tests and tracked in benchmarks without parsing stdout.

A multi-input solve used to split the input sets into equal static blocks per thread, plus a serial remainder on the calling thread. A heterogeneous sweep, such as a current sweep whose high currents stop early on a voltage cut-off, then waited on its slowest block. Sets are now handed out one at a time from a shared counter. The calling thread, which holds the GIL, always takes set 0, so its diagnostics still stream live. A failing set no longer hides the others: the error names every set that failed (input set 2: ...; input set 5: ...).

For pybammsolvers maintainers: pybamm now reads solution.stats unconditionally, so this bumps pybammsolvers to 0.10.0 and pins pybammsolvers>=0.10.0,<0.11.0. pybammsolvers-v0.10.0 has to be released before a pybamm release that contains this PR. #5783 (IDAS sensitivity scales) makes the identical version bump.

  • IDAKLUStats travels through SolutionData to Solution and is bound as idaklu.SolverStats. Statistics are saved on every solve, not only under print_stats. The BBD preconditioner's counter is read only when that preconditioner is in use; with preconditioner="none" SUNDIALS read it out of the model's own data, which also fixes a garbage value in print_stats.
  • IDAKLUSolverGroup runs #pragma omp parallel num_threads(team) over a std::atomic counter, rather than schedule(dynamic), which the macOS wheels' libomp and MSVC do not support. It still solves every set when the runtime starts fewer threads than asked (OMP_THREAD_LIMIT, nested regions). A Python exception such as KeyboardInterrupt stops new sets being taken, while an integrator failure is recorded against its set and the sweep continues.
  • StandaloneAlgebraicSystem owns its residual and Jacobian as unique_ptr<Expression>, with the CasADi constructor delegating to the Expression one, and Expression gains a virtual destructor.
  • IDAKLUSolver accepts a per-state atol as a list, tuple or array, so it survives a to_config/from_config round trip, and rejects one of the wrong width, booleans and non-real values with a SolverError.
  • SolverStatistics is documented in the API reference. Solutions pickled before this change still load.

This is one of the backend-neutral pre-flight PRs split out of the Rust execution engine (#5732). It has no Rust dependency. It overlaps #5783 (IDAS sensitivity scales) in IDAKLUSolverGroup.cpp and the IDAKLU tests, so whichever lands second needs a rebase.

Type of change

Add an entry under # [Unreleased] in CHANGELOG.md, in one of ## Breaking changes, ## Deprecated, ## Features, or ## Bug fixes (include PR number; breaking and deprecation entries need a one-line migration note). Internal-only PRs — refactor, docs, CI, tests — can skip this. See RELEASE.md for the full policy.

Important checks:

Please confirm the following before marking the PR as ready for review:

  • No style issues: nox -s pre-commit
  • All tests pass: nox -s tests
  • The documentation builds: nox -s doctests
  • Code is commented for hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.09%. Comparing base (0ef15f2) to head (a5d4fd3).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5782   +/-   ##
=======================================
  Coverage   98.08%   98.09%           
=======================================
  Files         346      346           
  Lines       35275    35323   +48     
=======================================
+ Hits        34599    34649   +50     
+ Misses        676      674    -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@martinjrobins martinjrobins 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.

thanks @BradyPlanden, looks great, minor suggestions below and (possible) missing stats on adding two solutions

aabills added a commit that referenced this pull request Sep 28, 2026
* Release v26.9.0.0 and pybammsolvers v0.10.0

Update CHANGELOG.md and CITATION.cff for v26.9.0.0. Merge the stray
second Breaking changes block into the first, drop the citation of the
unmerged #5782, and move the #5688 and #5630 entries out of v26.8.0.0,
which was tagged before they merged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Cite #5690 for the VTK plotting changelog entry

#5689 was closed unmerged; its VTK plotting work landed through #5690.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@BradyPlanden
BradyPlanden force-pushed the a2/idaklu-stats-scheduling branch from 5ba9f34 to 00495fe Compare September 30, 2026 10:44
@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

ProjectPyBaMM
Brancha2/idaklu-stats-scheduling
Testbedbare-metal

⚠️ WARNING: Truncated view!

The full continuous benchmarking report exceeds the maximum length allowed on this platform.

🐰 View full continuous benchmarking report in Bencher

martinjrobins
martinjrobins previously approved these changes Sep 30, 2026

@martinjrobins martinjrobins 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 great, thanks @BradyPlanden

@BradyPlanden

Copy link
Copy Markdown
Member Author

Found a few regressions on a second pass + incremented pybammsolvers version to avoid a broken pybamm release. Could you re-review @martinjrobins

…polymorphic Newton

Solutions carry IDAKLU's counters as SolverStatistics, summed as solutions combine; input
sets are self-scheduled with every failing set named; the standalone Newton owns its
Expressions; per-state atol takes lists and tuples. Requires pybammsolvers 0.10.0.
With 16 sets the GIL-free thread could finish all 15 of its sets before
thread 0 reached its first step on a loaded macOS runner.
Pass IDAKLUStats by const reference; suppress cppcheck where it cannot see
a use (the omp num_threads clause, members used only in the .cpp); mark
the tests' pickle round trips nosec, as main does.
Drop the num_threads() comment, which described the change rather than
the code, and name what the atomic counter stands in for.
…solutions

The __add__ short-circuit, and from_sub_solutions skipping the same
segment, dropped that operand's statistics. Its samples add nothing, but
the solver may still have worked for it.
A set whose integration fails part-way returns a partial solution with a
negative flag instead of throwing, so the per-set post-processing raised
on the first such set and hid the rest. Collect them all after the sweep,
and when another set throws, name the part-way failures alongside it, so
the SolverError and each on_failure="warn" warning carry "input set N: ".
An experiment solution is folded from the cycles it keeps, so the solver
work of a cycle that save_at_cycles leaves out went uncounted. Total those
cycles' statistics as they are dropped, keeping none of their solutions
alive, and add them to the folded solution's.
NumPy upcasts a bool beside numbers, so [True, 1e-6] became a float
array that passed the dtype check as a tolerance of 1.0.
The solver statistics, self-scheduled sweep and per-set failure messages
need a pybammsolvers newer than the released 0.10.0, so pin pybamm to 0.11.
A 0D domain's mass matrix block is dense, so a model whose differential
variables are all scalars or 0D handed block_diag only dense blocks,
which SciPy 1.18 deprecates in favour of returning a sparse array.
Convert each block to csr_matrix first.
The prebuilt-wheel path resolved pybamm's dependencies afresh with
uv pip install --no-sources, so CI tested the latest releases rather than
the locked set local nox sessions use. Sync the lock as usual, skipping
pybammsolvers, then install this interpreter's wheel with --no-deps.
pytest.warns records every warning raised in its block, not just the
matching ones, so an unrelated warning made re.match return None. Build
the model outside the block and ignore records that are not a
partial-solution warning.
@BradyPlanden
BradyPlanden force-pushed the a2/idaklu-stats-scheduling branch from cb77bfd to 06e7136 Compare September 30, 2026 15:53
@BradyPlanden
BradyPlanden merged commit b3c168f into main Oct 1, 2026
70 of 72 checks passed
@BradyPlanden
BradyPlanden deleted the a2/idaklu-stats-scheduling branch October 1, 2026 14:08
@rtimms rtimms mentioned this pull request Oct 9, 2026
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