From 9e6e1569cdf993037d1dca1b50eaa0043a82250b Mon Sep 17 00:00:00 2001 From: David Straub Date: Tue, 21 Jul 2026 22:59:41 +0200 Subject: [PATCH 1/5] Support for PyBaMM v26.7 --- .github/workflows/test.yml | 11 +++-- README.md | 2 +- pyproject.toml | 3 +- src/pathsim_batt/cells/pybamm_cell.py | 69 +++++++++++++++++++-------- tests/cells/test_lead_acid.py | 5 +- 5 files changed, 60 insertions(+), 30 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6be441f..baae32b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -31,16 +31,17 @@ jobs: matrix: include: # All Python versions against the latest PyBaMM - - {python-version: "3.10", pybamm-version: "26.6"} - - {python-version: "3.11", pybamm-version: "26.6"} - - {python-version: "3.12", pybamm-version: "26.6"} - - {python-version: "3.13", pybamm-version: "26.6"} - - {python-version: "3.14", pybamm-version: "26.6"} + - {python-version: "3.10", pybamm-version: "26.7"} + - {python-version: "3.11", pybamm-version: "26.7"} + - {python-version: "3.12", pybamm-version: "26.7"} + - {python-version: "3.13", pybamm-version: "26.7"} + - {python-version: "3.14", pybamm-version: "26.7"} # Older PyBaMM versions against the latest Python they support - {python-version: "3.13", pybamm-version: "25.12"} - {python-version: "3.14", pybamm-version: "26.3"} - {python-version: "3.14", pybamm-version: "26.4"} - {python-version: "3.14", pybamm-version: "26.5"} + - {python-version: "3.14", pybamm-version: "26.6"} steps: - uses: actions/checkout@v4 diff --git a/README.md b/README.md index 9f83033..dbe2802 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ Thermal sub-model and heat-source options are injected automatically — pass th | `lead_acid.Full` | `Sulzer2019` | ❌ DAE | ❌ DAE | ✅ | ✅ | | `equivalent_circuit.Thevenin` | `ECM_Example` | ✅ | ✅ | ✅ ² | ✅ ² | -¹ Pass `pybamm_solver=pybamm.CasadiSolver(mode="safe")` — the default `IDAKLUSolver` requires a Jacobian that ODE models do not provide. +¹ On PyBaMM < 26.7, pass `pybamm_solver=pybamm.CasadiSolver(mode="safe")` — the default `IDAKLUSolver` errors because `LOQS` disabled its Jacobian. Fixed upstream in PyBaMM 26.7; the default `IDAKLUSolver` works there. ² `initial_soc=1.0` fails because PyBaMM requires event values to be strictly positive at `t=0`; the "Maximum SoC" event is zero exactly at full charge. Any value below 1.0 (e.g. `initial_soc=0.99`) works. diff --git a/pyproject.toml b/pyproject.toml index 59bdc2e..3d3de5c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,7 +31,8 @@ dependencies = [ # marker resolves to a normal eager install on every real platform and # is skipped on Emscripten so the pure-Python parts of the toolbox # (`pathsim_batt.thermal`) still install in the browser. - "pybamm>=25.12; sys_platform != 'emscripten'", + # Upper-bounded: each new minor release is vetted before being allowed. + "pybamm>=25.12,<26.8; sys_platform != 'emscripten'", ] [project.optional-dependencies] diff --git a/src/pathsim_batt/cells/pybamm_cell.py b/src/pathsim_batt/cells/pybamm_cell.py index 7f6d516..397a5ea 100644 --- a/src/pathsim_batt/cells/pybamm_cell.py +++ b/src/pathsim_batt/cells/pybamm_cell.py @@ -133,32 +133,24 @@ def _detect_soc_direct_scale( return 1.0 / 100.0 if raw > 1.0 else 1.0 -def _inject_thermal_options( +def _inject_model_options( model: pybamm.BaseBatteryModel, required_options: dict[str, str], + guard_key: str | None = None, ) -> pybamm.BaseBatteryModel: """Return *model* with *required_options* merged into its options if needed. - Handles both the thermal sub-model selection (``"thermal": "isothermal"`` - or ``"lumped"``) and ancillary flags such as - ``"calculate heat source for isothermal models": "true"``. This means - users can pass a plain ``pybamm.lead_acid.LOQS()`` to - ``CellElectrothermal`` without having to specify ``thermal='lumped'`` - themselves — the block injects it automatically. - Models that already carry the required options are returned unchanged. - Models that have no ``"thermal"`` key in their options at all (e.g. ECM, - which manages temperature internally) are skipped silently — injection - does not apply to them. Models that expose ``"thermal"`` but whose - constructor rejects the specific option values emit a ``UserWarning`` - and are returned unchanged. + If *guard_key* is given and is not a key in *model*'s options at all + (e.g. ``"thermal"`` for ECM, which manages temperature internally, or + ``"voltage as a state"`` for models that don't expose that switch), + injection is skipped silently — it does not apply to them. Models that + expose the option(s) but whose constructor rejects the specific values + emit a ``UserWarning`` and are returned unchanged. """ if not required_options: return model - # Models that have no "thermal" key in their options (e.g. ECM) manage - # temperature through their own internal mechanism and do not use PyBaMM's - # thermal sub-model system. Injection is not applicable; skip silently. - if "thermal" in required_options and "thermal" not in model.options: + if guard_key is not None and guard_key not in model.options: return model if all(model.options.get(k) == v for k, v in required_options.items()): return model @@ -169,13 +161,47 @@ def _inject_thermal_options( warnings.warn( f"{type(model).__name__} does not support options {required_options}; " - "thermal behaviour may be incorrect.", + "behaviour may be incorrect.", UserWarning, stacklevel=4, ) return model +def _inject_thermal_options( + model: pybamm.BaseBatteryModel, + required_options: dict[str, str], +) -> pybamm.BaseBatteryModel: + """Return *model* with thermal *required_options* merged into its options. + + Handles both the thermal sub-model selection (``"thermal": "isothermal"`` + or ``"lumped"``) and ancillary flags such as + ``"calculate heat source for isothermal models": "true"``. This means + users can pass a plain ``pybamm.lead_acid.LOQS()`` to + ``CellElectrothermal`` without having to specify ``thermal='lumped'`` + themselves — the block injects it automatically. + + Models that have no ``"thermal"`` key in their options at all (e.g. ECM, + which manages temperature internally) are skipped silently. + """ + return _inject_model_options(model, required_options, guard_key="thermal") + + +# Since PyBaMM 26.7, these default to "true"/"algebraic", turning SPM/SPMe +# into DAEs; force the pre-26.7 pure-ODE discretisation that _CellBase needs. +_ODE_LEGACY_OPTIONS: dict[str, str] = { + "voltage as a state": "false", + "surface form": "false", +} + + +def _inject_ode_options(model: pybamm.BaseBatteryModel) -> pybamm.BaseBatteryModel: + """Force the pre-26.7 pure-ODE discretisation for models that support it.""" + return _inject_model_options( + model, _ODE_LEGACY_OPTIONS, guard_key="voltage as a state" + ) + + def _build_simulation( sim: pybamm.Simulation, model: pybamm.BaseBatteryModel, @@ -268,6 +294,7 @@ def __init__( model, {"thermal": self._thermal_option, **self._thermal_extra_options}, ) + model = _inject_ode_options(model) self._parameter_values = _prepare_parameter_values(parameter_values) try: @@ -280,7 +307,7 @@ def __init__( "and 'Upper voltage cut-off [V]'." ) from exc - pybamm_solver = pybamm_solver or pybamm.CasadiSolver(mode="safe") + pybamm_solver = pybamm_solver or pybamm.IDAKLUSolver() sim = pybamm.Simulation( model, @@ -605,7 +632,7 @@ class CellElectrical(_CellBase): Initial state of charge (0–1). Default 1.0. pybamm_solver : pybamm.BaseSolver or None PyBaMM solver used only during model build / discretisation. - Defaults to ``CasadiSolver(mode="safe")``. + Defaults to ``IDAKLUSolver()``. Inputs ------ @@ -654,7 +681,7 @@ class CellElectrothermal(_CellBase): Initial state of charge (0–1). Default 1.0. pybamm_solver : pybamm.BaseSolver or None PyBaMM solver used only during model build / discretisation. - Defaults to ``CasadiSolver(mode="safe")``. + Defaults to ``IDAKLUSolver()``. Inputs ------ diff --git a/tests/cells/test_lead_acid.py b/tests/cells/test_lead_acid.py index 0d06184..96d2a05 100644 --- a/tests/cells/test_lead_acid.py +++ b/tests/cells/test_lead_acid.py @@ -57,8 +57,9 @@ def test_electrothermal_smoke(self): assert_electrothermal_outputs(self, cell, self.v_lo, self.v_hi) def test_cosim_electrical_smoke(self): - # LOQS is an ODE; IDAKLUSolver (co-sim default) requires a Jacobian - # for ODE models and errors, so use CasadiSolver explicitly. + # On PyBaMM < 26.7, LOQS disables its Jacobian and IDAKLUSolver (the + # co-sim default) errors without one; CasadiSolver works on every + # pinned version (25.12-26.7), so use it explicitly here. solver = pybamm.CasadiSolver(mode="safe") cell = CellCoSimElectrical( model=self._model(), From 466115bd050f24054039d1a9cf5be3b11563dcdf Mon Sep 17 00:00:00 2001 From: David Straub Date: Tue, 21 Jul 2026 23:06:36 +0200 Subject: [PATCH 2/5] Use newer Python for mypy --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 3d3de5c..c690a31 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -72,6 +72,6 @@ select = ["E", "F", "W", "I"] [tool.mypy] files = ["src/pathsim_batt"] -python_version = "3.10" +python_version = "3.12" ignore_missing_imports = true warn_unused_ignores = true From 4baa8c8d1b29dfee59598c73617b982a5c139d10 Mon Sep 17 00:00:00 2001 From: David Straub Date: Tue, 21 Jul 2026 23:12:35 +0200 Subject: [PATCH 3/5] Fix --- src/pathsim_batt/cells/pybamm_cell.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pathsim_batt/cells/pybamm_cell.py b/src/pathsim_batt/cells/pybamm_cell.py index 397a5ea..35179b6 100644 --- a/src/pathsim_batt/cells/pybamm_cell.py +++ b/src/pathsim_batt/cells/pybamm_cell.py @@ -196,10 +196,10 @@ def _inject_thermal_options( def _inject_ode_options(model: pybamm.BaseBatteryModel) -> pybamm.BaseBatteryModel: - """Force the pre-26.7 pure-ODE discretisation for models that support it.""" - return _inject_model_options( - model, _ODE_LEGACY_OPTIONS, guard_key="voltage as a state" - ) + """Force the pre-26.7 pure-ODE discretisation for SPM/SPMe models.""" + if not isinstance(model, (pybamm.lithium_ion.SPM, pybamm.lithium_ion.SPMe)): + return model + return _inject_model_options(model, _ODE_LEGACY_OPTIONS) def _build_simulation( From b10e69dc20cc405fc8978025ce14477e5d555ca5 Mon Sep 17 00:00:00 2001 From: David Straub Date: Tue, 21 Jul 2026 23:20:01 +0200 Subject: [PATCH 4/5] Fix --- src/pathsim_batt/cells/pybamm_cell.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/pathsim_batt/cells/pybamm_cell.py b/src/pathsim_batt/cells/pybamm_cell.py index 35179b6..f6bf63a 100644 --- a/src/pathsim_batt/cells/pybamm_cell.py +++ b/src/pathsim_batt/cells/pybamm_cell.py @@ -187,19 +187,14 @@ def _inject_thermal_options( return _inject_model_options(model, required_options, guard_key="thermal") -# Since PyBaMM 26.7, these default to "true"/"algebraic", turning SPM/SPMe -# into DAEs; force the pre-26.7 pure-ODE discretisation that _CellBase needs. -_ODE_LEGACY_OPTIONS: dict[str, str] = { - "voltage as a state": "false", - "surface form": "false", -} - - +# "surface form" isn't a valid option outside SPM/SPMe, so it's only added there. def _inject_ode_options(model: pybamm.BaseBatteryModel) -> pybamm.BaseBatteryModel: - """Force the pre-26.7 pure-ODE discretisation for SPM/SPMe models.""" - if not isinstance(model, (pybamm.lithium_ion.SPM, pybamm.lithium_ion.SPMe)): + if "voltage as a state" not in model.options: return model - return _inject_model_options(model, _ODE_LEGACY_OPTIONS) + required = {"voltage as a state": "false"} + if isinstance(model, (pybamm.lithium_ion.SPM, pybamm.lithium_ion.SPMe)): + required["surface form"] = "false" + return _inject_model_options(model, required) def _build_simulation( From 0932c30a5f9e8875ad5084aa585088eb489892b6 Mon Sep 17 00:00:00 2001 From: David Straub Date: Tue, 21 Jul 2026 23:32:26 +0200 Subject: [PATCH 5/5] Fix --- README.md | 10 ++++---- src/pathsim_batt/cells/pybamm_cell.py | 2 +- tests/cells/test_lead_acid.py | 34 ++++++++++++++++++++++++--- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index dbe2802..cdba83b 100644 --- a/README.md +++ b/README.md @@ -73,13 +73,15 @@ Thermal sub-model and heat-source options are injected automatically — pass th | `lithium_ion.SPM` | `Chen2020` | ✅ | ✅ | ✅ | ✅ | | `lithium_ion.SPMe` | `Chen2020` | ✅ | ✅ | ✅ | ✅ | | `lithium_ion.DFN` | `Chen2020` | ❌ DAE | ❌ DAE | ✅ | ✅ | -| `lead_acid.LOQS` | `Sulzer2019` | ✅ | ✅ | ✅ ¹ | ✅ ¹ | +| `lead_acid.LOQS` | `Sulzer2019` | ✅ ¹ | ✅ ¹ | ✅ ² | ✅ ² | | `lead_acid.Full` | `Sulzer2019` | ❌ DAE | ❌ DAE | ✅ | ✅ | -| `equivalent_circuit.Thevenin` | `ECM_Example` | ✅ | ✅ | ✅ ² | ✅ ² | +| `equivalent_circuit.Thevenin` | `ECM_Example` | ✅ | ✅ | ✅ ³ | ✅ ³ | -¹ On PyBaMM < 26.7, pass `pybamm_solver=pybamm.CasadiSolver(mode="safe")` — the default `IDAKLUSolver` errors because `LOQS` disabled its Jacobian. Fixed upstream in PyBaMM 26.7; the default `IDAKLUSolver` works there. +¹ PyBaMM < 26.7 only — from 26.7 on `LOQS` is a DAE, use a `CellCoSim*` block instead. -² `initial_soc=1.0` fails because PyBaMM requires event values to be strictly positive at `t=0`; the "Maximum SoC" event is zero exactly at full charge. Any value below 1.0 (e.g. `initial_soc=0.99`) works. +² PyBaMM < 26.7 only — pass `pybamm_solver=pybamm.CasadiSolver(mode="safe")`; the default `IDAKLUSolver` errors on `LOQS`. Fixed in 26.7. + +³ `initial_soc=1.0` fails because PyBaMM requires event values to be strictly positive at `t=0`; the "Maximum SoC" event is zero exactly at full charge. Any value below 1.0 (e.g. `initial_soc=0.99`) works. ```python import pybamm diff --git a/src/pathsim_batt/cells/pybamm_cell.py b/src/pathsim_batt/cells/pybamm_cell.py index f6bf63a..5f2c296 100644 --- a/src/pathsim_batt/cells/pybamm_cell.py +++ b/src/pathsim_batt/cells/pybamm_cell.py @@ -156,7 +156,7 @@ def _inject_model_options( return model try: return type(model)(options={**dict(model.options), **required_options}) - except (pybamm.OptionError, TypeError): + except (pybamm.OptionError, TypeError, KeyError): import warnings warnings.warn( diff --git a/tests/cells/test_lead_acid.py b/tests/cells/test_lead_acid.py index 96d2a05..c76b45d 100644 --- a/tests/cells/test_lead_acid.py +++ b/tests/cells/test_lead_acid.py @@ -2,7 +2,7 @@ Block / model matrix covered ----------------------------- -lead_acid.LOQS — ODE → all 4 blocks +lead_acid.LOQS — ODE on PyBaMM < 26.7, DAE from 26.7 on → CoSim blocks only there lead_acid.Full — DAE → CoSim blocks only """ @@ -29,15 +29,24 @@ run_electrothermal, ) +# PyBaMM 26.7 registers "voltage as a state" centrally on every +# BaseBatteryModel (default "true"), and lead-acid models don't support +# disabling it, so LOQS is a DAE from 26.7 on and can no longer run in the +# monolithic (ODE-only) blocks. +_PYBAMM_VERSION = tuple(int(x) for x in pybamm.__version__.split(".")[:2]) +_LOQS_IS_ODE = _PYBAMM_VERSION < (26, 7) + # --------------------------------------------------------------------------- # lead_acid.LOQS (ODE — all 4 blocks) # --------------------------------------------------------------------------- class TestLeadAcidLOQS(unittest.TestCase): - """lead_acid.LOQS with Sulzer2019 parameters (ODE model — all blocks). + """lead_acid.LOQS with Sulzer2019 parameters. - Sulzer2019 cutoffs: lower 1.75 V, upper 2.42 V, nominal capacity 17 A·h. + ODE model (all 4 blocks) on PyBaMM < 26.7; DAE (CoSim blocks only) from + 26.7 on. Sulzer2019 cutoffs: lower 1.75 V, upper 2.42 V, nominal capacity + 17 A·h. """ def setUp(self): @@ -48,14 +57,28 @@ def setUp(self): def _model(self): return pybamm.lead_acid.LOQS() + @unittest.skipUnless(_LOQS_IS_ODE, "LOQS is a DAE on PyBaMM >= 26.7") def test_electrical_smoke(self): cell = run_electrical(self._model(), self.pv, current=17.0) assert_electrical_outputs(self, cell, self.v_lo, self.v_hi) + @unittest.skipUnless(_LOQS_IS_ODE, "LOQS is a DAE on PyBaMM >= 26.7") def test_electrothermal_smoke(self): cell = run_electrothermal(self._model(), self.pv, current=17.0) assert_electrothermal_outputs(self, cell, self.v_lo, self.v_hi) + @unittest.skipIf(_LOQS_IS_ODE, "LOQS is a pure ODE on PyBaMM < 26.7") + def test_monolithic_electrical_raises(self): + """LOQS is a DAE on PyBaMM >= 26.7 — CellElectrical must raise.""" + with self.assertRaises(NotImplementedError): + CellElectrical(model=self._model(), parameter_values=self.pv) + + @unittest.skipIf(_LOQS_IS_ODE, "LOQS is a pure ODE on PyBaMM < 26.7") + def test_monolithic_electrothermal_raises(self): + """LOQS is a DAE on PyBaMM >= 26.7 — CellElectrothermal must raise.""" + with self.assertRaises(NotImplementedError): + CellElectrothermal(model=self._model(), parameter_values=self.pv) + def test_cosim_electrical_smoke(self): # On PyBaMM < 26.7, LOQS disables its Jacobian and IDAKLUSolver (the # co-sim default) errors without one; CasadiSolver works on every @@ -103,16 +126,19 @@ def test_cosim_electrothermal_smoke(self): sim.run(2) assert_electrothermal_outputs(self, cell, self.v_lo, self.v_hi) + @unittest.skipUnless(_LOQS_IS_ODE, "LOQS is a DAE on PyBaMM >= 26.7") def test_electrical_soc_decreases(self): """SOC must decrease under discharge current.""" cell = run_electrical(self._model(), self.pv, current=17.0, duration=60) self.assertLess(float(cell.outputs[2]), 1.0) + @unittest.skipUnless(_LOQS_IS_ODE, "LOQS is a DAE on PyBaMM >= 26.7") def test_cutoff_values_match_parameter_set(self): cell = CellElectrical(model=self._model(), parameter_values=self.pv) self.assertAlmostEqual(cell._v_lower, self.v_lo) self.assertAlmostEqual(cell._v_upper, self.v_hi) + @unittest.skipUnless(_LOQS_IS_ODE, "LOQS is a DAE on PyBaMM >= 26.7") def test_q_dot_nonzero_during_discharge(self): """Q_dot must be strictly positive during discharge (isothermal LOQS). @@ -126,6 +152,7 @@ def test_q_dot_nonzero_during_discharge(self): "Q_dot is zero — thermal model may not compute heat sources", ) + @unittest.skipUnless(_LOQS_IS_ODE, "LOQS is a DAE on PyBaMM >= 26.7") def test_tamb_affects_temperature(self): """A warmer ambient temperature must yield a higher output cell temperature.""" solver = pybamm.CasadiSolver(mode="safe") @@ -156,6 +183,7 @@ def test_tamb_affects_temperature(self): ), ) + @unittest.skipUnless(_LOQS_IS_ODE, "LOQS is a DAE on PyBaMM >= 26.7") def test_soc_scale_factor(self): """SOC must be well below 1.0 after sustained discharge.