Solve the composite electrode SOH by bracketed rootfind - #5730
Draft
MarcBerliner wants to merge 3 commits into
Draft
Solve the composite electrode SOH by bracketed rootfind#5730MarcBerliner wants to merge 3 commits into
MarcBerliner wants to merge 3 commits into
Conversation
MarcBerliner
marked this pull request as draft
August 20, 2026 14:03
MarcBerliner
force-pushed
the
claude/composite-esoh-bracketed
branch
4 times, most recently
from
August 20, 2026 17:29
20626a7 to
238ea21
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## claude/brent-node #5730 +/- ##
=====================================================
- Coverage 98.08% 97.96% -0.13%
=====================================================
Files 341 341
Lines 32916 32993 +77
=====================================================
+ Hits 32285 32320 +35
- Misses 631 673 +42 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
MarcBerliner
force-pushed
the
claude/composite-esoh-bracketed
branch
from
August 21, 2026 16:23
238ea21 to
48e3612
Compare
MarcBerliner
force-pushed
the
claude/composite-esoh-bracketed
branch
from
August 21, 2026 16:43
48e3612 to
f2c5d45
Compare
MarcBerliner
added a commit
that referenced
this pull request
Aug 24, 2026
…ached Windows holds two copies of CasADi -- ours built with MSVC, the wheel's with MinGW -- so the plugin registered in one is invisible to the other, and the node refused to convert there at all. That took composite electrode SOH in #5730 with it, which works on Windows today. A CasADi Callback closes the gap. The residual is still compiled into the oracle exactly as the plugin path builds it; only the bracketing iteration runs in Python, driven by scipy.optimize.brentq. Everything around the rootfind stays in the graph, so a rootfind nested inside another costs one callback per enclosing iteration rather than a Python tree walk per residual evaluation: 200 nested solves take 0.26s this way against 0.004s through the plugin, where evaluating the tree directly could not finish one composite SOH solve in fourteen minutes. Derivatives come from the implicit function theorem, as the plugin's do: dx/dp = -(dF/dp) / (dF/dx) at the root, emitted as a forward mode over the same oracle. The tests confirm both paths agree to 1e-9 on a case with a known answer. Two things the driver cannot do. It re-enters Python, so the test that pins that out of the plugin path now skips when the plugin is absent, and it holds a callback, so an expression containing one cannot be code-generated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MarcBerliner
force-pushed
the
claude/composite-esoh-bracketed
branch
from
August 24, 2026 16:25
f2c5d45 to
d5d51d9
Compare
Each stoichiometry was a variable in one nine-variable algebraic system handed
to a Newton solver, which is a basin-of-attraction problem: worn and
lithium-poor states sat outside it. Now each is an expression, found by
inverting the open-circuit potential it sits on with a `pybamm.Brent`. The
OCPs diverge outside [0, 1], so every inversion brackets a root and converges
from any target, including targets outside the physical range, whose answer is
non-physical but exact.
Nothing is left for a solver to iterate, so the stoichiometries are read back
through one CasADi function rather than per-variable post-processing. The
capacities reach that function as inputs, so only the parameters its graph
substitutes can invalidate it: a caller ageing a cell reuses it, and a caller
swapping an open-circuit potential does not.
Over 8000 targets -- a thousand states of charge and a thousand voltages,
three hysteresis directions, four wear states:
main branch
failures 147/8000 0/8000
build 12.2 ms 18.5 ms
first solve 58.9 ms 14.8 ms
Main's failures are silent: it returns a state violating lithium conservation
rather than raising.
Repeat calls through a reused simulation, reading the stoichiometries back:
main branch
new target 18.8 ms 7.2 ms
aged capacities 15.3 ms 15.6 ms
no reuse 75.3 ms 91.2 ms
Reading the nine variables back through ProcessedVariable is still slower than
main, which the evaluator bypasses only for
get_initial_stoichiometries_composite.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The full solve evaluates the stoichiometries as one expression holding rootfinds nested about three deep -- one composite solve makes on the order of a million of them -- which only the native "brent" plugin gets through quickly. Windows cannot load that plugin, and neither Python-side route is usable there: walking the tree could not finish a single solve in fourteen minutes, and driving the compiled residual from a CasADi callback still pays 830k Python round trips. So Windows takes the path it took before this branch. solve_split is algebraic throughout -- _ElectrodeSOH through a Simulation, no rootfind node anywhere in it -- and get_initial_stoichiometries_composite already falls back to it, so raising from solve_full is all it takes. 47 of the 63 tests in the file pass that way in 35s; the 16 that exercise the full solve itself skip, since what they test is genuinely absent. pybamm.is_windows() joins is_macos_intel() in util, so the check has one home. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test_reaches_the_requested_state solves ElectrodeSOHComposite as a model and reads its variables back, so the rootfinds travel into IDAKLU as a serialised function. On Windows they are a CasADi Callback by then, which the solver's own CasADi cannot deserialise -- "not found 'CallbackInternal'" -- so this is the same gap as the other full-solve tests, one layer further in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MarcBerliner
force-pushed
the
claude/composite-esoh-bracketed
branch
from
August 24, 2026 18:47
d5d51d9 to
3a8b1da
Compare
MarcBerliner
marked this pull request as ready for review
August 24, 2026 20:53
aabills
reviewed
Aug 24, 2026
| Q_p.append(pybamm.InputParameter("Q_p_2")) | ||
|
|
||
| T_ref = param.T_ref | ||
| T_init = param.T_init if initialization_method == "voltage" else T_ref |
Contributor
There was a problem hiding this comment.
is this logic necessary? Kinda strange that we change the value of temperature based on initialization method
MarcBerliner
marked this pull request as draft
September 3, 2026 20:07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #5729, which adds
pybamm.Brent. Review that first; this diff is against it.What changes
Each stoichiometry was a variable in one nine-variable algebraic system handed to a Newton solver, which makes reaching a state a basin-of-attraction problem — worn and lithium-poor cells sat outside it.
Now each stoichiometry is an expression, found by inverting the open-circuit potential it sits on with a
pybamm.Brent.LithiumIonParameters.Uclips its stoichiometry and adds an asymptote, so every OCP diverges outside[0, 1]; every inversion therefore brackets a root and converges from any target — including targets outside the physical range, whose answer is non-physical but exact.Nothing is left for a solver to iterate, so the stoichiometries are read back through one CasADi function rather than per-variable post-processing. The capacities reach that function as inputs, so only the parameters its graph substitutes can invalidate it: a caller ageing a cell reuses it, a caller swapping an OCP does not.
Measured against
main8000 targets — 1000 states of charge on
[0, 1], 1000 voltages on[2.5, 4.2] V, three hysteresis directions, four wear states, through the public API:mainmain's failures are silent — every one returns a state violating lithium conservation rather than raising, concentrated at low SOC.Repeat calls through a reused simulation, reading the stoichiometries back:
mainKnown limitations
ProcessedVariableis slower thanmain(roughly 4×). It converts and evaluates the whole nested rootfind once per variable; the evaluator bypasses that, but only forget_initial_stoichiometries_composite, which is the pathSimulationuses. A caller drivingElectrodeSOHCompositedirectly pays it.solve_splitis kept as the fallback and is now actually reachable — the native rootfinder reports failure asRuntimeError, which the old ladder did not catch, and the retry re-ran an identical solve. It now returns the split result instead.[0, 1]would still bracket, and Brent would return one of its roots.Chen2020_compositedescribes a composite negative. That is a degenerate composite, not a real parameter set.Tests: 4221 unit, 32 sweep tests in
test_electrode_soh_composite_sweeps.py.🤖 Generated with Claude Code