Coupled GITT-EIS parameterisation - #975
Open
Ombrini wants to merge 31 commits into
Open
Conversation
…ality - Introduced a new example script for parameter estimation from GITT experiments with operando EIS. - Enhanced the EISSimulator class to support operando simulations and added methods for handling impedance data. - Updated the WeightedCost class to maintain distinct targets for different cost functions. - Added unit tests for the operando EIS simulator to ensure correct functionality and output.
…able handling, and enhance plotting capabilities
…e operando EIS file, and update method names for clarity
Ombrini
marked this pull request as ready for review
August 12, 2026 16:27
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
Adds coupled GITT-EIS parameterisation: the ability to fit a model against a
time-domain voltage trace and impedance spectra measured at chosen points along that
trace, in a single optimisation.
The motivating case is a GITT experiment with a spectrum acquired at the end of each
pulse, where the relaxation constrains transport and the charge-transfer arc constrains
kinetics — two parameters that are hard to separate from either measurement alone.
Fixes # (issue)
How it works
The design leans on the fact that the cost layer is domain-agnostic but length-strict.
So both data types live in one
pybop.Dataseton theTime [s]domain:Time [s],Current [A],Voltage [V]— full lengthn_time2 x n_frequencyreal-valued impedance variables, also lengthn_time, non-zero onlyat the rows where a spectrum was acquired.
Real and imaginary components are separate real variables rather than one complex
variable. Their names come from a single helper so the dataset and the
simulator cannot disagree:
EISSimulatorgains aprotocolargument. It infers the acquisition times from the rowswhere the impedance variables are non-zero, and per evaluation it solves the time-domain
trajectory once, then linearises about the state at each of those times. Because
everything shares the
Time [s]domain, the two cost measures combine with the existingpybop.WeightedCost, which provides the relative weighting — no new cost class:Bug fixes included
Input parameters were stacked in the wrong order (pre-existing). PyBaMM orders a built
model's input parameters alphabetically (
base_solver.py,ordered_inputs_names = sorted(...)), but the EIS code stacked them in the caller's dict order:With one fitted parameter this cannot go wrong, which is why it went unnoticed. With two
whose dict order is not alphabetical, the values are swapped and the model is evaluated at
the wrong point — in the new example, diffusivity and the exchange-current prefactor were
exchanged. Values are now sorted by name before stacking.
Note: the same pattern exists in PyBaMM's own
EISSimulation._build_matrix_problem, so itis worth reporting upstream.
WeightedCostsub-costs lost their individual targets.Problem.__init__callsset_target(None), which fell through totarget or self._target, whereself._targetisthe flattened union of the sub-cost targets. Its first element is a string, so it was
broadcast to every cost, and each cost then evaluated all targets. Now
target is Nonekeepseach cost's own target.
Also in this PR
_initialise_eis_matricespreviously re-ransolver.set_up()and rebuilt the mass matrix and forcing vector on every evaluation,although neither depends on the inputs or the operating point. These are now set up once,
mirroring
Simulator._set_up_solution_method.set_up_for_eisno longer mutates the caller's model — it works onmodel.new_copy(),matching PyBaMM's
EISSimulation. Previously a user'sSPM()came back with two extraalgebraic equations.
"surface form": "differential"the double layer isabsent and the computed impedance is silently meaningless (a near-pure resistance). This
now raises, as PyBaMM's
EISSimulationdoes. This caught an existing test(
test_fitting_problem_eis) that was asserting against such a spectrum.pybop.plot.problemnow recognises impedance variables and draws them asone figure of Nyquist subplots — one per acquisition, equal aspect ratio — instead of one
near-empty time series per variable. Adds
equal_aspectto the plot backends.examples/scripts/battery_parameterisation/operando_eis.pyis deleted: itdated from an earlier attempt, used the previous output shape, and is superseded by
gitt_eis.py.Simulator.time_data, added in the same earlier attempt and referencednowhere, is also removed.
pybop.pybamm.EISSimulator.set_output_variablesis deliberately a no-op, with a commentexplaining why: restricting the solver to output variables makes PyBaMM stop returning the
state vector (
sol.ycomes back with shape(0, N)), and the state vector is exactly whatthe linearisation needs.
New example
examples/scripts/battery_parameterisation/gitt_eis.py— simulates a GITT experiment,computes a spectrum about the state at the end of each pulse, assembles the combined
dataset, and recovers the positive-electrode diffusivity and exchange-current prefactor.
The latter is a
FunctionParameterin Chen2020 with a hard-coded prefactor, so the exampleredefines the function with the prefactor exposed, following
examples/scripts/getting_started/functional_parameters.py.Type of change
New feature, plus two bug fixes. Not a breaking change to existing user code, with two
caveats worth calling out in review:
EISSimulatornow raises for models without a surface form, where it previouslyreturned a (meaningless) result.
PlotBackendgains an abstract method, which breaks any third-party backend subclass.Important checks:
ruff checkandruff formatclean overpybop/,tests/,examples/nox -s tests— 542 passed, 26 skipped, with one failure thatalso occurs on
developwithout this branch's changes (see below)nox -s doctest— passed (1 passed, 549 skipped)New tests (
tests/unit/test_simulator.py::TestCoupledEISSimulator):test_output_shape_and_zero_paddingn_time, non-zero only at acquisitionstest_matches_stationary_at_initial_statet=0equals a stationary simulation's (exact,rtol=1e-10) — fails if the linearisation is wired up wrongtest_builds_oncetest_model_is_not_modifiedset_up_for_eiscopies the modeltest_surface_form_requiredtest_impedance_variables_round_triptest_dataset_errorsPlus
tests/unit/test_cost.py::test_weighted_cost_keeps_distinct_targetsfor theWeightedCostfix.Pre-existing failure, unrelated to this PR
tests/unit/test_pybamm_utils.py::TestPybammUtils::test_simulate_procedurefails withConfirmed pre-existing: stashing every change on this branch and re-running the test
reproduces the same failure. It looks like a Polars compatibility problem — the same run
emits
DeprecationWarning: & on Boolean and Int16 is deprecated and will raise a ComputeError in Polars 2.0frompybop/applications/utils.py:85. Worth a separate issue.Test changes required by the surface-form validation
Running the full suite showed the new validation catching two more places that were
computing EIS without a double layer, both fixed here:
tests/unit/test_pybamm_utils.py::test_simulate_procedurebuilds the model for a set ofprocedures that includes an EIS sweep, so it now uses
SPM(options={"surface form": "differential"})— matching whatexamples/scripts/synthetic_data/generate_synthetic_data.pyalready does.tests/integration/models/test_grouped_models.py::test_eis_fittingskips the two modelconfigurations without a surface form. Its
model_configfixture is shared with teststhat legitimately use plain models, so the skip is local to the EIS test.
Open questions for review
The example's
weights=[1.0, 1e3]is a starting point chosen so that the two terms arecomparable in magnitude, not a derived value.
The spectra are placed at the end of each rest (current zero, cell relaxed) rather
than at the end of the current pulse. A non-zero current at an acquisition time warns
rather than fails, since the Jacobian is still valid about that operating point — but the
quasi-steady assumption behind EIS is only approximately met while the cell is still
relaxing.
pybop.plot.problemrecognises impedance variables by their name format. That is aconvention, not a type, so a dataset built without
get_impedance_variableswill not beplotted as Nyquist.