Skip to content

Update IDAKLUSolver error handling#5291

Merged
MarcBerliner merged 10 commits intodevelopfrom
update-solver-errors
Dec 1, 2025
Merged

Update IDAKLUSolver error handling#5291
MarcBerliner merged 10 commits intodevelopfrom
update-solver-errors

Conversation

@MarcBerliner
Copy link
Copy Markdown
Member

@MarcBerliner MarcBerliner commented Nov 24, 2025

Description

Updates the IDAKLUSolver error handling to always throw pybamm.SolverError, and use pybammsolvers' function for generating error messages

Requires pybamm-team/pybammsolvers#75 and pybammsolvers v0.3.3

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #)

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

@MarcBerliner MarcBerliner marked this pull request as ready for review November 24, 2025 20:36
@MarcBerliner MarcBerliner requested a review from a team as a code owner November 24, 2025 20:36
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.77%. Comparing base (593d2aa) to head (60f1bb7).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #5291      +/-   ##
===========================================
- Coverage    98.77%   98.77%   -0.01%     
===========================================
  Files          321      321              
  Lines        27747    27746       -1     
===========================================
- Hits         27407    27406       -1     
  Misses         340      340              

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Copy Markdown
Member

@BradyPlanden BradyPlanden left a comment

Choose a reason for hiding this comment

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

LGTM - thanks!

@MarcBerliner MarcBerliner merged commit df53205 into develop Dec 1, 2025
22 checks passed
@MarcBerliner MarcBerliner deleted the update-solver-errors branch December 1, 2025 16:34
BradyPlanden pushed a commit that referenced this pull request Dec 8, 2025
* raise `SolverError` at failure to init sundials

* Update simulation.py

* Update idaklu_solver.py

* reuse `pybammsolvers` error messages

* Update test_idaklu_solver.py

* bump `pybammsolvers`

* Update CHANGELOG.md

* Update CHANGELOG.md

Update CHANGELOG.md
rtimms added a commit that referenced this pull request Dec 16, 2025
* Don't be too strict with func_args longer than symbol.children

* Add a test

* Add support for uniform grid sizing across subdomains (#720) (#5253)

Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com>

* Fix typo in Butler-Volmer equation docstring (#5279)

* fix bug with bulk ocp lithiation (#5280)

* doc: fix typo in concentration description in notebook (#5284)

* Fix typo in concentration description in notebook

* Add CHANGELOG.md entry for typo fix

* Remove unneccesary changelog entry

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

---------

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* fix: instruct uv to install into system for CI (#5288)

* Fix `InputParameter` serialisation (#5289)

* fix `InputParameter` serialisation

* Update CHANGELOG.md

* Bugfix: inputs for `initial_conditions_from` scale evaluation (#5285)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Add `silence_sundials_errors` solver option (#5290)

* feat: add`silence_sundial_warnings` solver option

* refactor: `silence_sundials_warnings` -> `silence_sundials_errors`

* Update C-Rate current for changing nominal capacity (#5286)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Update `IDAKLUSolver` error handling (#5291)

* raise `SolverError` at failure to init sundials

* Update simulation.py

* Update idaklu_solver.py

* reuse `pybammsolvers` error messages

* Update test_idaklu_solver.py

* bump `pybammsolvers`

* Update CHANGELOG.md

* Update CHANGELOG.md

Update CHANGELOG.md

* #5299 add inverse linear kinetics

* #5299 check domain options for inverse kinetics

* update notebook

* #5299 brady comments

* #5299 fix composite total kinetics

---------

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Co-authored-by: Swasti Mishra <140950062+swastim01@users.noreply.github.com>
Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com>
Co-authored-by: Chase Naples <cnaples79@gmail.com>
Co-authored-by: Gregor Decristoforo <gregor.decristoforo@gmail.com>
Co-authored-by: Pip Liggins <philippa.liggins@dtc.ox.ac.uk>
Co-authored-by: Marc Berliner <34451391+MarcBerliner@users.noreply.github.com>
Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
MarcBerliner added a commit that referenced this pull request Dec 17, 2025
…#5308)

* Don't be too strict with func_args longer than symbol.children

* Add a test

* Add support for uniform grid sizing across subdomains (#720) (#5253)

Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com>

* Fix typo in Butler-Volmer equation docstring (#5279)

* fix bug with bulk ocp lithiation (#5280)

* doc: fix typo in concentration description in notebook (#5284)

* Fix typo in concentration description in notebook

* Add CHANGELOG.md entry for typo fix

* Remove unneccesary changelog entry

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

---------

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* fix: instruct uv to install into system for CI (#5288)

* Fix `InputParameter` serialisation (#5289)

* fix `InputParameter` serialisation

* Update CHANGELOG.md

* Bugfix: inputs for `initial_conditions_from` scale evaluation (#5285)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Add `silence_sundials_errors` solver option (#5290)

* feat: add`silence_sundial_warnings` solver option

* refactor: `silence_sundials_warnings` -> `silence_sundials_errors`

* Update C-Rate current for changing nominal capacity (#5286)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Update `IDAKLUSolver` error handling (#5291)

* raise `SolverError` at failure to init sundials

* Update simulation.py

* Update idaklu_solver.py

* reuse `pybammsolvers` error messages

* Update test_idaklu_solver.py

* bump `pybammsolvers`

* Update CHANGELOG.md

* Update CHANGELOG.md

Update CHANGELOG.md

* revive `SymbolReplacer`

* add `VariableReplacementMap`

* Create replace_symbols.rst

* Update index.rst

* cleanup

* attach `parameter_values` to model for lazy post-processing

* update `model.parameter_values` serialization

* Update test_serialisation.py

* Update test_parameter_values.py

* add `observe` to solution for lazy evaluation of discretized symbols

* Update test_solution.py

* style

* Update CHANGELOG.md

* fix docstrings

* docs

* docs2

* Update replace_symbols.py

* Update replace_symbols.py

* remove `SymbolReplacer`

* add slots to `SpatialMethod` for memory

* add `SymbolProcessor` for `BaseModel` symbol eval

* add `ModelSolutionObservability` enums for `BaseModel`

* parameter_values: delay variable processing

* fix stale symbol id: use `scale` and `reference` setters

* discretisation delayed var processing, don't overwrite `model.variables`

* pass `delayed_variable_processing` args through `process_model`

* add `DUMMY_INPUT_PARAMETER_VALUE` const

* Update __init__.py

* Update index.rst

* Delete replace_symbols.py

* update symbol `scale` and `reference` setters

* update variable hash

* explicit `_variables_processed` base model attribute

* use `get_processed_variable_or_event` getter

* copy model in `Simulation`

* disable observability with `output_variables`

* delay variable processing in `Simulation` build

* add `Solution.observe(symbol)`

* update serialization

* update examples

* Update CONTRIBUTING.md

* Update CHANGELOG.md

* add observe tests

* update serialization tests

* Update test_base_model.py

* update symbol processor getters

* add model observability tests

* update symbol and base variable id setters

* Update symbol_processor.py

* Update CHANGELOG.md

* fix model serialization notebook

* Update symbol_processor.py

* remove unused serialisation methods

* remove debug statement

* update observe test comments

* add observe test failure for new parameters

* remove `_solution_observable` from custom model serialization

* fix observe test error

* set `DUMMY_INPUT_PARAMETER_VALUE = np.nan` to make errors more obvious

---------

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Co-authored-by: Swasti Mishra <140950062+swastim01@users.noreply.github.com>
Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com>
Co-authored-by: Chase Naples <cnaples79@gmail.com>
Co-authored-by: Robert Timms <43040151+rtimms@users.noreply.github.com>
Co-authored-by: Gregor Decristoforo <gregor.decristoforo@gmail.com>
Co-authored-by: Pip Liggins <philippa.liggins@dtc.ox.ac.uk>
Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
martinjrobins added a commit that referenced this pull request Mar 7, 2026
* Don't be too strict with func_args longer than symbol.children

* Add a test

* Add support for uniform grid sizing across subdomains (#720) (#5253)

Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com>

* Fix typo in Butler-Volmer equation docstring (#5279)

* fix bug with bulk ocp lithiation (#5280)

* doc: fix typo in concentration description in notebook (#5284)

* Fix typo in concentration description in notebook

* Add CHANGELOG.md entry for typo fix

* Remove unneccesary changelog entry

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

---------

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* fix: instruct uv to install into system for CI (#5288)

* Fix `InputParameter` serialisation (#5289)

* fix `InputParameter` serialisation

* Update CHANGELOG.md

* Bugfix: inputs for `initial_conditions_from` scale evaluation (#5285)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Add `silence_sundials_errors` solver option (#5290)

* feat: add`silence_sundial_warnings` solver option

* refactor: `silence_sundials_warnings` -> `silence_sundials_errors`

* Update C-Rate current for changing nominal capacity (#5286)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Update `IDAKLUSolver` error handling (#5291)

* raise `SolverError` at failure to init sundials

* Update simulation.py

* Update idaklu_solver.py

* reuse `pybammsolvers` error messages

* Update test_idaklu_solver.py

* bump `pybammsolvers`

* Update CHANGELOG.md

* Update CHANGELOG.md

Update CHANGELOG.md

* implement heat equation export

* Move DiffslExport to evaluate_python.py

Co-authored-by: martinjrobins <martinjrobins@gmail.com>

* add tests for DiffSLExport

* rename DiffslExport to DiffSLExport

* consider both options for heat flux output

* move diffsl code to new file

* add spm, spme and dfn tests, apply fixes for these

* can export dfn and spme, but output is incorrect

* new input syntax

* move model export to integration, tidy up float formattting

* update changelog

* update to new delayed variable pipeline

* minor fixes, remove dead code

* turn off pydiffsol dep for mac intel

* don't use inplace for integration tests

* improve coverage

* fix: reg functions

* doc: minor comment to explain function fallback

* refactor: eqn -> eqn_str

* refactor: use ordered dict, remove unnecessary code

* test: remove diffsl integration tests

* test: add snapshot tests for unit diffsl export tests

* test: reg_power and arcsinh2

* dep: remove pydiffsol

---------

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Co-authored-by: Swasti Mishra <140950062+swastim01@users.noreply.github.com>
Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com>
Co-authored-by: Chase Naples <cnaples79@gmail.com>
Co-authored-by: Robert Timms <43040151+rtimms@users.noreply.github.com>
Co-authored-by: Gregor Decristoforo <gregor.decristoforo@gmail.com>
Co-authored-by: Pip Liggins <philippa.liggins@dtc.ox.ac.uk>
Co-authored-by: Marc Berliner <34451391+MarcBerliner@users.noreply.github.com>
Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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.

2 participants