Skip to content

Add IDAKLU upsampling_factor for scalar output resolution - #5998

Open
MarcBerliner wants to merge 6 commits into
mainfrom
claude/linear-upsampling-experiment
Open

MarcBerliner wants to merge 6 commits into
mainfrom
claude/linear-upsampling-experiment

Conversation

@MarcBerliner

@MarcBerliner MarcBerliner commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Description

Adds the IDAKLUSolver option upsampling_factor. When it is above 1.0, the solver saves extra points inside each accepted step until linear interpolation of one scalar output matches the integrator's own interpolant to tol / (factor - 1), where tol = atol + rtol·|v|. So 1 is off, 2 is about the solver tolerance, and larger values add more points. Points are only ever added.

How it works (inline, in the C++ step loop)

  • The scalar variable is generated with the existing process() codegen, the same path as output_variables, and passed to the solver group as upsampling_fcn(t, y, p).
  • After each accepted step, the value at t_val is evaluated once from the state already loaded and reused as the next step's start. The value at the step midpoint comes from IDAGetDky, written into y_cache. If |(v_a + v_b)/2 − v_m| misses the budget, the step is bisected and each midpoint is saved through the normal SavePoint.
  • No derivatives are used. It works with or without stored Hermite data, with output_variables, and with sensitivities. Sensitivities at added points are IDA's own interpolated values.
  • Experiment steps choose the variable through upsampling_variable: "Voltage [V]" by default, "Current [A]" for voltage steps. It is attached to the built model at build time. The unified model keeps the voltage default.
  • Algebraic states are skipped, because IDA's error test already covers them. In a voltage hold the current is the algebraic state, so upsampling does nothing in that step.
  • It is mutually exclusive with hermite_reduction_factor > 1, and inactive when t_interp is given.

SPMe (Chen2020) benchmarks, fastest of 15 warm solves:

config 1C discharge 5-step experiment + 2 sensitivities
off 2.2 ms, 99 pts 17.5 ms, 407 pts 33.5 ms
factor 1.1 +0.9 ms, 110 pts +2.6 ms, 420 pts +2.7 ms
factor 2 +1.2 ms, 148 pts +4.3 ms, 505 pts +4.5 ms
factor 21 +4.0 ms, 443 pts +11.8 ms, 1383 pts +14.6 ms

Max linear-interpolation error of V between saved points, relative to the solver tolerance, on the 1C discharge: 350 (off), 13 (1.1), 2.5 (2), 0.42 (5), 0.08 (21).

SPMe 1C discharge: upsampling_factor sweep

A cubic Hermite version that also used the analytic dV/dt was explored first (5e86f72e4) and replaced. It needed fewer points for the same error, but cost about twice as much per solve at loose budgets.

create_casadi_solver_group gains a required upsampling_fcn argument, so this needs the matching pybammsolvers build.

Type of change

  • New feature: CHANGELOG entry under ## Features.

Important checks:

  • No style issues: pre-commit run on changed files
  • Tests pass locally: packages/pybamm/tests/unit/test_solvers, test_experiments and test_simulation.py (932 passed); the pybammsolvers tests (70 passed)
  • The documentation builds: nox -s doctests (not run)
  • Tests added: TestUpsampling in test_idaklu_solver.py, and test_upsampling_variable in test_base_step.py

Inside each accepted step, bisect and save extra points until the cubic
Hermite interpolant of a scalar output (built from its value and analytic
time derivative) matches the integrator's interpolant to
solver_tol / (factor - 1). Points are only added. Works with or without
stored Hermite data, with output_variables and with sensitivities.

Experiment steps choose the output (voltage, or current for voltage
steps) via hermite_upsampling_variable, carried on the built model as
hermite_upsampling_variables; algebraic states are skipped.
Prototype: check the midpoint against linear interpolation between step
ends instead of the cubic Hermite, and skip the analytic time derivative.
Check each step midpoint against linear interpolation of the upsampled
variables' values, so the function needs only (t, y, p): no analytic
time derivative and no yp interpolation. Rename hermite_upsampling_* to
upsampling_* (option, function, step and model attributes).
The upsampled variable is now one scalar (model.upsampling_variable),
passed to C++ as the function from process(). Midpoints interpolate into
y_cache so the step-end state in yy needs no copy or restore, and the
redundant t_val > t_prev guard is dropped.
@MarcBerliner
MarcBerliner marked this pull request as ready for review October 28, 2026 18:15
@MarcBerliner
MarcBerliner requested a review from a team as a code owner October 28, 2026 18:15
@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

ProjectPyBaMM
Branchclaude/linear-upsampling-experiment
Testbedbare-metal

⚠️ WARNING: Truncated view!

The full continuous benchmarking report exceeds the maximum length allowed on this platform.

🐰 View full continuous benchmarking report in Bencher

@codecov

codecov Bot commented Oct 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.23%. Comparing base (5b81c32) to head (2ef8b64).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5998   +/-   ##
=======================================
  Coverage   98.23%   98.23%           
=======================================
  Files         350      350           
  Lines       36104    36127   +23     
=======================================
+ Hits        35468    35491   +23     
  Misses        636      636           

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

This branch has not been deployed

No deployments
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.

1 participant