Repository navigation
fix: make CasADi's RegPower and uniform-grid interpolants match evaluate - #5955
Merged
BradyPlanden merged 1 commit intoOct 28, 2026
Merged
Conversation
This was referenced Oct 22, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## stack/00-function-copy-fix #5955 +/- ##
==============================================================
- Coverage 98.25% 98.23% -0.02%
==============================================================
Files 350 350
Lines 36109 36104 -5
==============================================================
- Hits 35478 35468 -10
- Misses 631 636 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
| Project | PyBaMM |
| Branch | stack/00b-casadi-numerics-fix |
| Testbed | bare-metal |
🐰 View full continuous benchmarking report in Bencher
⚠️ WARNING: Truncated view!The full continuous benchmarking report exceeds the maximum length allowed on this platform.
MarcBerliner
previously approved these changes
Oct 23, 2026
Comment on lines
+292
to
+293
| # CasADi's "exact" lookup can pick a neighbouring segment beside a knot | ||
| lookup_modes = ["binary"] * len(self.x) |
Member
There was a problem hiding this comment.
Is there anything simple we can do to keep the exact lookups? It's not a big deal but it'd be nice to use them if we can instead of the slower binary search
RegPower's CasADi form, x * hypot(x, delta)^(a-1) * scale^a, rounded differently from evaluate's x * (x^2 + delta^2)^((a-1)/2) * scale^a. Both now evaluate one sequence of operations, and a literal exponent of 0.5 or 2 takes its powers as square roots, so CasADi and evaluate agree bitwise for those exponents. Linear and pchip interpolants on a uniform grid used CasADi's "exact" lookup, which can pick the neighbouring segment for a point at or one ulp beside a knot, so the Jacobian took that segment's slope. They now use binary search, as on a non-uniform grid.
BradyPlanden
force-pushed
the
stack/00b-casadi-numerics-fix
branch
from
October 28, 2026 09:55
67e54b0 to
a72301b
Compare
rtimms
approved these changes
Oct 28, 2026
4 of 5 tasks
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.
Description
CasADi evaluated
RegPower, and linear and pchip interpolants on a uniform grid, differently fromevaluate. Both changes reach every user on the default CasADi path.RegPower:evaluatecomputedx * (x² + δ²)^((a−1)/2) * scale^a, withx = base / scale, while CasADi computedx * hypot(x, δ)^(a−1) * scale^a, so the two rounded differently.RegPower._decomposition, each with its ownsqrt,powerand constants. The Python/JAX evaluator callsevaluate's form, whose_array_sqrttraces JAX arrays.x / sqrt(sqrt(x² + δ²)) * sqrt(scale)for 0.5), so CasADi andevaluateagree bitwise, and for 0.5 the relative error stays below6·2⁻⁵³.delta == 0,_decompositionreturnspower(base, exponent), soevaluate, JAX and CasADi all take the plain power._function_diffand_function_jacdifferentiatebase**exponentthere too. Before, CasADi alone kept this fallback:evaluategave NaN atbase = 0with exponent 0.5, and −16 rather than 16 atbase = −4with exponent 2, and the symbolic Jacobian disagreed with CasADi's values for negative bases.powform for everydeltaand exponent, unchanged._is_constant_value, which only the old CasADi form used, is removed.xby binary search on every grid."exact"lookup. That can pick the neighbouring segment at a knot or one ulp beside it, and so put that segment's slope in the Jacobian._is_uniform_gridis removed.#5959 lowers
RegPowerto Rust through the same_decomposition, and its parity tests hold Rust to these forms. This PR uses nothing from the Rust stack and can merge on its own.Type of change
Bug fix, under
## Bug fixesinCHANGELOG.md.Key checklist
pre-commit rundelta = 0tests fail before this change.RegPowerbitwise equal toevaluate, for exponents 0.5 and 2 at scales 1 and 33133;RegPowerat 0.5 against a 60-digitDecimalreference;delta = 0, values and the Jacobian against CasADi atbase0, −4 and 4, for exponents 0.5 and 2;Rust execution engine stack: proposal #5667, showcase #5732
Each PR is one commit on the one above it.
Function.create_copykeeps the function's classRegPowerand uniform-grid interpolant lookups matchevaluate← this PRpybamm-core: expression graph, typed IR, interpreters, forward-mode ADpybamm-core:CompiledModel, coloured Jacobians, the C ABIpybamm.rustbindings, built into thepybammwheelSymbol.to_rustconvert_to_format = "rust"forScipySolver,AlgebraicSolverand EISpybammsolvers: IDAKLU over the C ABIIDAKLUSolveron rust modelspybamm-core: diffsol DAE solverpybamm.DiffsolSolver"rust"as the defaultconvert_to_format