Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5347 +/- ##
===========================================
- Coverage 98.47% 98.39% -0.09%
===========================================
Files 326 326
Lines 28519 28661 +142
===========================================
+ Hits 28085 28201 +116
- Misses 434 460 +26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…m-team/PyBaMM into more-robust-composite-esoh
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
…m-team/PyBaMM into more-robust-composite-esoh
…m-team/PyBaMM into more-robust-composite-esoh
BradyPlanden
left a comment
There was a problem hiding this comment.
Just a few general comments
src/pybamm/models/full_battery_models/lithium_ion/electrode_soh.py
Outdated
Show resolved
Hide resolved
src/pybamm/models/full_battery_models/lithium_ion/electrode_soh_composite.py
Outdated
Show resolved
Hide resolved
src/pybamm/models/full_battery_models/lithium_ion/electrode_soh_composite.py
Outdated
Show resolved
Hide resolved
src/pybamm/models/full_battery_models/lithium_ion/electrode_soh_composite.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR improves the robustness of the ElectrodeSOHComposite model by implementing a split solve approach as a fallback mechanism, and ensures consistent calculation of equilibrium stoichiometries for models with hysteresis. The split solve first computes primary phase stoichiometries using a simplified model, then solves for secondary phase stoichiometries by enforcing equal OCP across phases. When the full solve fails, the code automatically falls back to the split solve for initial conditions and retries.
Changes:
- Added
_get_equilibrium_direction()helper function to correctly determine OCP branch for equilibrium stoichiometries (100% SOC uses charging branch, 0% SOC uses discharging branch) - Implemented
solve_split()andsolve_full()static methods inElectrodeSOHCompositewith automatic fallback mechanism - Updated
_ElectrodeSOHandElectrodeSOHCompositeto use equilibrium directions for stoichiometry limits, making them temperature-independent and consistent with BPX definitions
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pybamm/models/full_battery_models/lithium_ion/util.py | Added _get_equilibrium_direction() function to determine correct OCP branch for equilibrium stoichiometries based on SOC state and hysteresis presence |
| src/pybamm/models/full_battery_models/lithium_ion/electrode_soh_composite.py | Implemented split solve methodology with solve_split() and solve_full() methods, added fallback mechanism in get_initial_stoichiometries_composite(), and updated equilibrium stoichiometry calculations |
| src/pybamm/models/full_battery_models/lithium_ion/electrode_soh.py | Updated to use _get_equilibrium_direction() for calculating stoichiometry limits, ensuring consistency with hysteresis models |
| tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_electrode_soh.py | Expanded test coverage with more initial values, added tests for split solve mechanism, and tests for hysteresis handling |
| CHANGELOG.md | Documented the improvement to ElectrodeSOHComposite robustness and equilibrium stoichiometry consistency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/pybamm/models/full_battery_models/lithium_ion/electrode_soh_composite.py
Show resolved
Hide resolved
src/pybamm/models/full_battery_models/lithium_ion/electrode_soh_composite.py
Show resolved
Hide resolved
src/pybamm/models/full_battery_models/lithium_ion/electrode_soh_composite.py
Outdated
Show resolved
Hide resolved
src/pybamm/models/full_battery_models/lithium_ion/electrode_soh_composite.py
Outdated
Show resolved
Hide resolved
src/pybamm/models/full_battery_models/lithium_ion/electrode_soh_composite.py
Outdated
Show resolved
Hide resolved
src/pybamm/models/full_battery_models/lithium_ion/electrode_soh_composite.py
Outdated
Show resolved
Hide resolved
…h_composite.py Co-authored-by: Marc Berliner <34451391+MarcBerliner@users.noreply.github.com>
BradyPlanden
left a comment
There was a problem hiding this comment.
Looks good to me - thanks!
* minor composite esoh fixes * style: pre-commit fixes * try split composite eSOH solve * use split for initial conditions on failure * style: pre-commit fixes * fix v_low/high vs soc_0/100 * consisten min/max sto calculations * update changelog * style: pre-commit fixes * PR comments * fix direction in solve_split * Update src/pybamm/models/full_battery_models/lithium_ion/electrode_soh_composite.py Co-authored-by: Marc Berliner <34451391+MarcBerliner@users.noreply.github.com> --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Marc Berliner <34451391+MarcBerliner@users.noreply.github.com> (cherry picked from commit 18461a1)
Description
The current
ElectrodeSOHCompositeis not very robust and often gives solver failures. This adds a "split solve" method that first solves the standard eSOH model using the "primary" OCV curves and total capacities, then solves that all the OCP must be equal in all phases in the electrode at equilibrium. Theset_initial_statemethod now tries to solve the full eSOH model. If it fails, it uses the "split" model (which is only approximately correct) to obtain initial conditions, then retries solving the full eSOH model. This improves the robustness considerably based on the test cases I have run.This PR also ensures the
_0and_100stoichiometry variables are defined consistently when a model contains hysteresis. Specifically, it implements the same definition as used in BPX: "Stoichiometry limits (x_0, x_100, y_0, y_100) are evaluated in a temperature-independent manner (at the reference temperature, ignoring entropy effects), so that the SOC range is not temperature-dependent. In the presence of a hysteresis model, equilibration in the charging direction is assumed for 100% SOC (charging OCP branch), and equilibration in the discharging direction is assumed for 0% SOC (discharging OCP branch)."Fixes # (issue)
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:
nox -s pre-commitnox -s testsnox -s doctests