Skip to content

Conversation

@aaryan-iiitu
Copy link

Summary

This PR fixes the interpolation bound errors occurring in the sodium-ion DFN model (Chayambuka2022 parameter set), where OCP and exchange-current interpolants would exceed their defined stoichiometry or concentration ranges during deep-discharge conditions. This caused solver failures such as:

  • "Interpolant 'k_n' lower bound"
  • "Interpolant 'U_n' lower bound"

Fix

I updated the sodium-ion parameter functions to enable extrapolate=True for:

  • Negative electrode OCP
  • Negative electrode electrolyte exchange current density
  • Positive electrode OCP
  • Positive electrode electrolyte exchange current density

This ensures that DFN simulations do not crash when stoichiometry or concentration slightly exceeds tabulated data ranges.

Tests

Added a regression test test_sodium_ion_extrapolation.py that:

  • Loads the Chayambuka2022 parameter set
  • Retrieves the modified interpolant functions
  • Asserts that they all have extrapolate=True

This test runs quickly and confirms the fix without requiring long DFN simulations.

Motivation

The sodium-ion DFN demo fails to complete cycles in recent PyBaMM versions due to strict interpolation bounds. This PR improves robustness while matching expected physical behavior.


agriyakhetarpal and others added 18 commits November 13, 2025 09:12
… (pybamm-team#5253)

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

* 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 `InputParameter` serialisation

* Update CHANGELOG.md
…rialisation-fix

Don't be too strict with func_args longer than symbol.children
…-team#5285)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* feat: add`silence_sundial_warnings` solver option

* refactor: `silence_sundials_warnings` -> `silence_sundials_errors`
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* 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
@aaryan-iiitu aaryan-iiitu requested a review from a team as a code owner December 4, 2025 05:35
@rtimms
Copy link
Contributor

rtimms commented Dec 5, 2025

extrapolate is True by default, so I'm surprised this changes anything. Can you share a script and outputs comparing leaving the default and explicitly passing extrapolate=True? I think what is probably happening is that in your test you aren't solving for a long enough time or high enough rate to see the error.

@aaryan-iiitu
Copy link
Author

Thanks for the review .You are completely correct. I have re ran the tests on the current branch without my changes, and I am unable to reproduce the crash as the simulation completes successfully.It appears the default handling in the current codebase is already robust enough to prevent it.

Since I cannot reproduce the failure locally, I understand this PR might be redundant. However, I am still very keen to work on resolving the original issue for the user. Could you guide me on how to solve this issue if possible , otherwise I would be happy to close the PR.

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.

10 participants