From 47d0a9288b38001046e0117692e3c67041bebcc7 Mon Sep 17 00:00:00 2001 From: bradyplanden Date: Thu, 1 Oct 2026 10:18:09 +0100 Subject: [PATCH 1/6] perf: store Solution attributes in slots An experiment keeps four solutions per step (the step, its cycle and the cycle's first and last states), and each carried a full-size __dict__. Solution set more instance attributes than CPython's 30-key shared layout holds, and Python 3.14 pre-fills those keys from __static_attributes__, so the layout also depended on which solutions a process had made before. Slots give every solution the same compact layout. first_state and last_state cache in slots instead of through cached_property, __dict__ stays for attributes callers attach, and __setstate__ loads solutions pickled before Solution had slots. --- .../pybamm/src/pybamm/solvers/solution.py | 86 +++++++++++++++++-- .../tests/unit/test_solvers/test_solution.py | 66 +++++++++++++- 2 files changed, 143 insertions(+), 9 deletions(-) diff --git a/packages/pybamm/src/pybamm/solvers/solution.py b/packages/pybamm/src/pybamm/solvers/solution.py index 6440e76cd2..d82de8b323 100644 --- a/packages/pybamm/src/pybamm/solvers/solution.py +++ b/packages/pybamm/src/pybamm/solvers/solution.py @@ -7,7 +7,6 @@ import numbers import pickle from dataclasses import astuple, dataclass -from functools import cached_property from itertools import chain import casadi @@ -96,6 +95,10 @@ class SolutionBase: across solution types. """ + # An experiment keeps several solutions per step; __dict__ stays so callers + # can still attach their own attributes + __slots__ = ("__dict__", "__weakref__", "_data", "set_up_time", "solve_time") + def __init__(self): self.set_up_time = None self.solve_time = None @@ -297,8 +300,45 @@ class Solution(SolutionBase): """ - # A class default, so solutions pickled before this attribute existed load - solver_statistics: SolverStatistics | None = None + __slots__ = ( + "_all_inputs_casadi", + "_all_inputs_stacked", + "_all_models", + "_all_sensitivities", + "_all_t_evals", + "_all_ts", + "_all_yps", + "_all_ys", + "_all_ys_and_sens", + "_cycles", + "_first_state", + "_initial_start_time", + "_last_state", + "_observable", + "_options", + "_sensitivities", + "_sub_solutions", + "_summary_variables", + "_t", + "_t_eval", + "_t_event", + "_termination", + "_user_options", + "_variables", + "_y", + "_y_event", + "_yp", + "all_first_states", + "all_inputs", + "all_summary_variables", + "closest_event_idx", + "cycle_summary_variables", + "extrap_events", + "integration_time", + "solver_statistics", + "steps", + "variables_returned", + ) def __init__( self, @@ -379,6 +419,7 @@ def __init__( super().__init__() self.integration_time = None + self.solver_statistics = None self._all_inputs_stacked = None self._all_inputs_casadi = None @@ -406,6 +447,9 @@ def __init__( # Initialise initial start time self.initial_start_time = None + self._first_state = None + self._last_state = None + # Check no ys are too large if check_solution: self.check_ys_are_not_too_large() @@ -413,6 +457,28 @@ def __init__( # Solution now uses CasADi pybamm.citations.register("Andersson2019") + def __setstate__(self, state: dict | tuple) -> None: + """Restore a pickled solution. + + Parameters + ---------- + state : dict or tuple + The ``(instance dict, slots)`` pair a solution pickles, or the + single dict of a solution pickled before ``Solution`` had slots. + """ + # An older pickle may lack attributes added since, and caches the + # first and last states under their property names + self.solver_statistics = None + self._first_state = None + self._last_state = None + if isinstance(state, tuple): + instance_state, slot_state = state + state = {**(instance_state or {}), **(slot_state or {})} + for name, value in state.items(): + if name in ("first_state", "last_state"): + name = f"_{name}" + setattr(self, name, value) + def has_sensitivities(self) -> bool: return len(self._all_sensitivities) > 0 @@ -639,13 +705,18 @@ def user_options(self) -> dict: def options(self) -> dict: return self._options - @cached_property + @property def first_state(self): """ A Solution object that only contains the first state. This is faster to evaluate than the full solution when only the first state is needed (e.g. to initialize a model with the solution) """ + if self._first_state is None: + self._first_state = self._build_first_state() + return self._first_state + + def _build_first_state(self): sensitivities = {} n_states = self.all_models[0].len_rhs_and_alg for key in self._all_sensitivities: @@ -684,13 +755,18 @@ def first_state(self): return new_sol - @cached_property + @property def last_state(self): """ A Solution object that only contains the final state. This is faster to evaluate than the full solution when only the final state is needed (e.g. to initialize a model with the solution) """ + if self._last_state is None: + self._last_state = self._build_last_state() + return self._last_state + + def _build_last_state(self): sensitivities = {} n_states = self.all_models[-1].len_rhs_and_alg for key in self._all_sensitivities: diff --git a/packages/pybamm/tests/unit/test_solvers/test_solution.py b/packages/pybamm/tests/unit/test_solvers/test_solution.py index e7e5f79cdd..d372773aa8 100644 --- a/packages/pybamm/tests/unit/test_solvers/test_solution.py +++ b/packages/pybamm/tests/unit/test_solvers/test_solution.py @@ -727,6 +727,46 @@ def test_cycles(self): np.testing.assert_array_equal(sol.cycles[1].t, sol.t[len_cycle_1:]) np.testing.assert_allclose(sol.cycles[1].y, sol.y[:, len_cycle_1:]) + def test_retained_solutions_keep_their_attributes_in_slots(self): + # An experiment keeps several solutions per step, so none of them should + # need an instance __dict__ + experiment = pybamm.Experiment( + [("Discharge at 1C for 5 minutes", "Rest for 5 minutes")] * 2 + ) + sol = pybamm.Simulation(pybamm.lithium_ion.SPM(), experiment=experiment).solve() + retained = [sol, *sol.cycles, *sol.sub_solutions] + retained += [cycle.first_state for cycle in sol.cycles] + retained += [cycle.last_state for cycle in sol.cycles] + for solution in retained: + assert vars(solution) == {} + + @pytest.mark.skipif( + sys.version_info < (3, 13), reason="__static_attributes__ is new in 3.13" + ) + def test_attributes_set_by_methods_are_slots(self): + # An attribute a method sets without a slot would give every solution + # a __dict__ again + classes = pybamm.Solution.__mro__[:-1] + slots = {name for cls in classes for name in cls.__slots__} + properties = { + name + for cls in classes + for name, value in vars(cls).items() + if isinstance(value, property) + } + for cls in classes: + assert set(cls.__static_attributes__) <= slots | properties + + def test_pickle_keeps_slots_and_extra_attributes(self): + t = np.linspace(0, 1) + solution = pybamm.Solution(t, np.tile(t, (2, 1)), pybamm.BaseModel(), {}) + first_state = solution.first_state + solution.label = "extra" + loaded = pickle.loads(pickle.dumps(solution)) # nosec B301 + np.testing.assert_array_equal(loaded.t, t) + np.testing.assert_array_equal(loaded.first_state.y, first_state.y) + assert loaded.label == "extra" + def test_total_time(self): sol = pybamm.Solution(np.array([0]), np.array([[1, 2]]), pybamm.BaseModel(), {}) sol.set_up_time = 0.5 @@ -1942,8 +1982,26 @@ def test_statistics_survive_pickling(self): solution = self._solution(0, statistics) assert pickle.loads(pickle.dumps(solution)).solver_statistics == statistics # nosec B301 - def test_solution_pickled_without_statistics_loads(self): + def test_solution_pickled_before_slots_loads(self): solution = self._solution(0, pybamm.SolverStatistics(1, 2, 3, 4, 5)) - # A solution pickled before the attribute existed has no such state - del solution.__dict__["solver_statistics"] - assert pickle.loads(pickle.dumps(solution)).solver_statistics is None # nosec B301 + first_state = solution.first_state + # A solution pickled before Solution had slots holds one dict, without + # solver_statistics and with the cached first state under its name + state = { + name: getattr(solution, name) + for cls in type(solution).__mro__ + for name in getattr(cls, "__slots__", ()) + if not name.startswith("__") and hasattr(solution, name) + } + del state["solver_statistics"], state["_first_state"], state["_last_state"] + state["first_state"] = first_state + + class PreSlotsPickle: + def __reduce__(self): + return object.__new__, (pybamm.Solution,), state + + loaded = pickle.loads(pickle.dumps(PreSlotsPickle())) # nosec B301 + assert loaded.solver_statistics is None + np.testing.assert_array_equal(loaded.t, solution.t) + np.testing.assert_array_equal(loaded.first_state.y, first_state.y) + assert loaded.last_state.t[0] == pytest.approx(solution.t[-1]) From 4022f37bb05b65eaad424cd722f74b8376948b27 Mon Sep 17 00:00:00 2001 From: bradyplanden Date: Thu, 1 Oct 2026 10:18:10 +0100 Subject: [PATCH 2/6] fix(pybammsolvers): find CasADi from an editable install idaklu's only CasADi rpath is ../casadi, which holds for a wheel beside casadi in site-packages but not for an editable install, whose extension lives in the build tree. import pybammsolvers then failed to load libcasadi unless casadi had been imported first. An editable build also searches the environment's CasADi directory. --- packages/pybammsolvers/CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/pybammsolvers/CMakeLists.txt b/packages/pybammsolvers/CMakeLists.txt index 579692239d..af58ccc45b 100644 --- a/packages/pybammsolvers/CMakeLists.txt +++ b/packages/pybammsolvers/CMakeLists.txt @@ -205,6 +205,11 @@ if (${USE_PYTHON_CASADI}) "${SuiteSparse_ROOT}/lib64" ) endif() + # An editable install loads idaklu from the build tree, which has no + # ../casadi, so it also searches the environment's CasADi directory + if (SKBUILD_STATE STREQUAL "editable") + list(APPEND IDAKLU_INSTALL_RPATH "${CASADI_LIB_DIR}") + endif() set_target_properties(idaklu PROPERTIES INSTALL_RPATH "${IDAKLU_INSTALL_RPATH}" BUILD_WITH_INSTALL_RPATH TRUE From a093664118efea50cc95aee276e2d8d6820c6085 Mon Sep 17 00:00:00 2001 From: bradyplanden Date: Thu, 1 Oct 2026 10:18:11 +0100 Subject: [PATCH 3/6] test: tighten the cycle-count memory gate to 1.3 With solutions in slots the 20-to-5 cycle ratio is about 1.25 on Python 3.13 and 1.28 on 3.14 when run after the rest of the memory suite, so a 1.4 gate no longer caught a regression the size of main's 1.39. --- packages/pybamm/tests/memory/test_memory_leaks.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/pybamm/tests/memory/test_memory_leaks.py b/packages/pybamm/tests/memory/test_memory_leaks.py index 063bc669a6..cf538b38ab 100644 --- a/packages/pybamm/tests/memory/test_memory_leaks.py +++ b/packages/pybamm/tests/memory/test_memory_leaks.py @@ -220,12 +220,12 @@ def test_cycle_count_memory_scaling(self): mem_20_cycles, _ = tracemalloc.get_traced_memory() tracemalloc.stop() - # Linear growth (4x) means each step rebuilds the model. On a ~1.9 MB fixed - # footprint, ~48 KB/cycle growth gives ~1.37x, so 1.4 bounds allocator noise. + # Linear growth (4x) means each step rebuilds the model. On a ~1.86 MB fixed + # footprint, ~33 KB/cycle growth gives ~1.25x, so 1.3 bounds allocator noise. ratio = mem_20_cycles / mem_5_cycles - assert ratio < 1.4, ( - f"Memory grew {ratio:.1f}x for 4x more cycles. " - f"Expected sub-linear growth (<1.4x). " + assert ratio < 1.3, ( + f"Memory grew {ratio:.2f}x for 4x more cycles. " + f"Expected sub-linear growth (<1.3x). " f"This may indicate termination hashing is broken (see #5453)." ) From 4916e95cb2e481de8df82da9cf843b20613ec821 Mon Sep 17 00:00:00 2001 From: bradyplanden Date: Thu, 1 Oct 2026 10:19:21 +0100 Subject: [PATCH 4/6] docs: add the changelog entries for #5827 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74fe907b1b..9d918799f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,9 +19,11 @@ - `IDAKLUSolver`'s `print_stats` with an iterative linear solver and `preconditioner="none"` no longer reports a garbage "Number of calls to residual function in preconditioner": without a BBD preconditioner, SUNDIALS read that count out of the model's own data. ([#5782](https://github.com/pybamm-team/PyBaMM/pull/5782)) - Discretising a model whose differential variables are all scalars or on zero-dimensional domains (such as a lumped current collector) no longer raises SciPy 1.18's `block_diag` `DeprecationWarning`, and its mass matrix stays a sparse matrix rather than becoming a sparse array once SciPy changes `block_diag`'s return type. ([#5782](https://github.com/pybamm-team/PyBaMM/pull/5782)) - The CI test matrix now installs every dependency from `uv.lock`, as local nox sessions do, instead of resolving the latest releases, so only the prebuilt `pybammsolvers` wheel is installed outside the lock. ([#5782](https://github.com/pybamm-team/PyBaMM/pull/5782)) +- An editable `pybammsolvers` install, such as the one `uv sync` creates, can be imported on its own: `import pybammsolvers` no longer fails to load `libcasadi` unless `casadi` was imported first. ([#5827](https://github.com/pybamm-team/PyBaMM/pull/5827)) ## Optimizations +- `Solution` stores its attributes in `__slots__`, so an experiment, which keeps four solutions per step, retains about 40% less memory per step on Python 3.13. Solutions keep a `__dict__` for attributes callers attach, though `vars(solution)` no longer lists the built-in ones, and solutions pickled by earlier versions still load. ([#5827](https://github.com/pybamm-team/PyBaMM/pull/5827)) - Symbol mutation is no longer guarded by intercepting every attribute write, and `children` and `domain` return the stored immutable sequences instead of building a view on each access. Writes to a symbol's slots after construction are instead rejected by a static check of PyBaMM's code, tests, examples and documentation. Under the test suite's mutation guard, constructing an expression is twice as fast and parameterising it is 25% faster, which recovers most of the `test_parameterise` benchmark regression from [#5779](https://github.com/pybamm-team/PyBaMM/pull/5779). ([#5825](https://github.com/pybamm-team/PyBaMM/pull/5825)) # [v26.9.0.0](https://github.com/pybamm-team/PyBaMM/tree/pybamm-v26.9.0.0) - 2026-09-28 From b54e51ba7b3d00bfdc8af59d0e8d887c7b50d467 Mon Sep 17 00:00:00 2001 From: bradyplanden Date: Thu, 1 Oct 2026 10:45:40 +0100 Subject: [PATCH 5/6] fix: load EIS solutions pickled before solutions had slots SolutionBase's slots took _data, set_up_time and solve_time out of the instance dict, but only Solution restored a pre-slots pickle. An older EISSolution put them in __dict__, where the slots hid them, so its impedance, frequencies, data and total_time raised AttributeError. SolutionBase now restores every pickled attribute through setattr, and Solution keeps only its own defaults and renamed state caches. --- .../pybamm/src/pybamm/solvers/solution.py | 37 +++++++++++-------- .../pybamm/tests/unit/test_eis_simulation.py | 19 ++++++++++ 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/packages/pybamm/src/pybamm/solvers/solution.py b/packages/pybamm/src/pybamm/solvers/solution.py index d82de8b323..95c9ae025b 100644 --- a/packages/pybamm/src/pybamm/solvers/solution.py +++ b/packages/pybamm/src/pybamm/solvers/solution.py @@ -104,6 +104,22 @@ def __init__(self): self.solve_time = None self._data = pybamm.FuzzyDict() + def __setstate__(self, state: dict | tuple) -> None: + """Restore a pickled solution. + + Parameters + ---------- + state : dict or tuple + The ``(instance dict, slots)`` pair a solution pickles, or the + single dict of a solution pickled before solutions had slots. + """ + # A plain dict would otherwise go into __dict__, where slots hide it + if isinstance(state, tuple): + instance_state, slot_state = state + state = {**(instance_state or {}), **(slot_state or {})} + for name, value in state.items(): + setattr(self, name, value) + def __getitem__(self, key): """Access a variable by name.""" return self._data[key] @@ -458,26 +474,17 @@ def __init__( pybamm.citations.register("Andersson2019") def __setstate__(self, state: dict | tuple) -> None: - """Restore a pickled solution. - - Parameters - ---------- - state : dict or tuple - The ``(instance dict, slots)`` pair a solution pickles, or the - single dict of a solution pickled before ``Solution`` had slots. - """ # An older pickle may lack attributes added since, and caches the # first and last states under their property names self.solver_statistics = None self._first_state = None self._last_state = None - if isinstance(state, tuple): - instance_state, slot_state = state - state = {**(instance_state or {}), **(slot_state or {})} - for name, value in state.items(): - if name in ("first_state", "last_state"): - name = f"_{name}" - setattr(self, name, value) + if isinstance(state, dict): + state = { + f"_{name}" if name in ("first_state", "last_state") else name: value + for name, value in state.items() + } + super().__setstate__(state) def has_sensitivities(self) -> bool: return len(self._all_sensitivities) > 0 diff --git a/packages/pybamm/tests/unit/test_eis_simulation.py b/packages/pybamm/tests/unit/test_eis_simulation.py index 4584c71315..ab0724ed19 100644 --- a/packages/pybamm/tests/unit/test_eis_simulation.py +++ b/packages/pybamm/tests/unit/test_eis_simulation.py @@ -1,3 +1,5 @@ +import pickle # nosec B403 - used in tests with trusted input + import numpy as np import pytest @@ -146,6 +148,23 @@ def test_eis_solution_save_pickle(self, tmp_path): loaded = pybamm.load(str(filepath)) np.testing.assert_array_equal(loaded.impedance, z) + def test_eis_solution_pickled_before_slots_loads(self): + freqs = np.array([1.0, 10.0]) + z = np.array([1 + 0.5j, 2 + 1j]) + data = pybamm.EISSolution(freqs, z).data + # A solution pickled before solutions had slots holds one dict + state = {"_data": data, "set_up_time": 0.5, "solve_time": 1.5} + + class PreSlotsPickle: + def __reduce__(self): + return object.__new__, (pybamm.EISSolution,), state + + loaded = pickle.loads(pickle.dumps(PreSlotsPickle())) # nosec B301 + np.testing.assert_array_equal(loaded.impedance, z) + np.testing.assert_array_equal(loaded.frequencies, freqs) + assert loaded.data.keys() == data.keys() + assert loaded.total_time == pytest.approx(2.0) + def test_eis_solution_save_data_invalid_format(self, tmp_path): sol = pybamm.EISSolution(np.array([1.0]), np.array([1 + 1j])) with pytest.raises(ValueError, match="Unrecognised format"): From 423ed384d0c70a9ca6e25aa2e5839061819411a3 Mon Sep 17 00:00:00 2001 From: bradyplanden Date: Thu, 1 Oct 2026 13:08:41 +0100 Subject: [PATCH 6/6] feat (breaking): remove __dict__ from Solution class --- CHANGELOG.md | 3 +- .../pybamm/src/pybamm/solvers/solution.py | 22 +++++++--- .../tests/unit/test_solvers/test_solution.py | 40 +++++++++---------- 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d918799f3..d145aa2381 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Breaking changes - `Symbol.children`, `Symbol.domain` (and `secondary_domain`, `tertiary_domain`, `quaternary_domain`) and `FunctionParameter.input_names` return immutable tuples that compare equal to lists, and `Symbol.domains` returns a read-only copy, as do `to_json()`, `get_children_domains()`, `read_domain_or_domains()` and `pybamm.EMPTY_DOMAINS`. Editing them raises `TypeError` instead of emitting `SymbolMutationDeprecationWarning`, and not even `list` or `dict` methods can change a symbol through them. This ends the deprecation of these edits early; before 26.9 they left the symbol's identity stale. The sequences are no longer `list` instances and slicing them gives a plain tuple; tuples are accepted wherever domains are given. Build a new symbol with `with_domains`, `create_copy(new_children=...)` or `pybamm.replace`. The deprecated setters (`name`, `domains`, `scale`, ...) are unchanged. ([#5825](https://github.com/pybamm-team/PyBaMM/pull/5825)) +- `Solution` and `EISSolution` no longer have an instance `__dict__`, so attributes they do not define cannot be attached to them. Subclass `pybamm.Solution` to add attributes. A solution pickled with an attached attribute still loads, without that attribute and with a logged warning. ([#5827](https://github.com/pybamm-team/PyBaMM/pull/5827)) ## Deprecated @@ -23,7 +24,7 @@ ## Optimizations -- `Solution` stores its attributes in `__slots__`, so an experiment, which keeps four solutions per step, retains about 40% less memory per step on Python 3.13. Solutions keep a `__dict__` for attributes callers attach, though `vars(solution)` no longer lists the built-in ones, and solutions pickled by earlier versions still load. ([#5827](https://github.com/pybamm-team/PyBaMM/pull/5827)) +- `Solution` stores its attributes in `__slots__`, so an experiment, which keeps four solutions per step, retains about 40% less memory per step on Python 3.13. Solutions pickled by earlier versions still load. ([#5827](https://github.com/pybamm-team/PyBaMM/pull/5827)) - Symbol mutation is no longer guarded by intercepting every attribute write, and `children` and `domain` return the stored immutable sequences instead of building a view on each access. Writes to a symbol's slots after construction are instead rejected by a static check of PyBaMM's code, tests, examples and documentation. Under the test suite's mutation guard, constructing an expression is twice as fast and parameterising it is 25% faster, which recovers most of the `test_parameterise` benchmark regression from [#5779](https://github.com/pybamm-team/PyBaMM/pull/5779). ([#5825](https://github.com/pybamm-team/PyBaMM/pull/5825)) # [v26.9.0.0](https://github.com/pybamm-team/PyBaMM/tree/pybamm-v26.9.0.0) - 2026-09-28 diff --git a/packages/pybamm/src/pybamm/solvers/solution.py b/packages/pybamm/src/pybamm/solvers/solution.py index 95c9ae025b..ff4d2aa489 100644 --- a/packages/pybamm/src/pybamm/solvers/solution.py +++ b/packages/pybamm/src/pybamm/solvers/solution.py @@ -95,9 +95,9 @@ class SolutionBase: across solution types. """ - # An experiment keeps several solutions per step; __dict__ stays so callers - # can still attach their own attributes - __slots__ = ("__dict__", "__weakref__", "_data", "set_up_time", "solve_time") + # An experiment keeps several solutions per step, so solutions have no + # __dict__; set_up_time and solve_time are set by solvers and simulations + __slots__ = ("__weakref__", "_data", "set_up_time", "solve_time") def __init__(self): self.set_up_time = None @@ -113,12 +113,20 @@ def __setstate__(self, state: dict | tuple) -> None: The ``(instance dict, slots)`` pair a solution pickles, or the single dict of a solution pickled before solutions had slots. """ - # A plain dict would otherwise go into __dict__, where slots hide it if isinstance(state, tuple): instance_state, slot_state = state state = {**(instance_state or {}), **(slot_state or {})} + dropped = [] for name, value in state.items(): - setattr(self, name, value) + try: + setattr(self, name, value) + except AttributeError: + dropped.append(name) + if dropped: + pybamm.logger.warning( + f"Dropped attributes {dropped} that {type(self).__name__} " + "does not define when loading a pickled solution" + ) def __getitem__(self, key): """Access a variable by name.""" @@ -202,6 +210,8 @@ class EISSolution(SolutionBase): Complex impedance values at each frequency. """ + __slots__ = () + def __init__(self, frequencies, impedance): super().__init__() impedance = np.asarray(impedance, dtype=complex) @@ -316,6 +326,8 @@ class Solution(SolutionBase): """ + # Solvers, Simulation and make_cycle_solution set all_first_states, closest_event_idx, + # cycle_summary_variables, extrap_events, integration_time, solver_statistics and steps __slots__ = ( "_all_inputs_casadi", "_all_inputs_stacked", diff --git a/packages/pybamm/tests/unit/test_solvers/test_solution.py b/packages/pybamm/tests/unit/test_solvers/test_solution.py index d372773aa8..f78c23461a 100644 --- a/packages/pybamm/tests/unit/test_solvers/test_solution.py +++ b/packages/pybamm/tests/unit/test_solvers/test_solution.py @@ -727,25 +727,22 @@ def test_cycles(self): np.testing.assert_array_equal(sol.cycles[1].t, sol.t[len_cycle_1:]) np.testing.assert_allclose(sol.cycles[1].y, sol.y[:, len_cycle_1:]) - def test_retained_solutions_keep_their_attributes_in_slots(self): - # An experiment keeps several solutions per step, so none of them should - # need an instance __dict__ - experiment = pybamm.Experiment( - [("Discharge at 1C for 5 minutes", "Rest for 5 minutes")] * 2 - ) - sol = pybamm.Simulation(pybamm.lithium_ion.SPM(), experiment=experiment).solve() - retained = [sol, *sol.cycles, *sol.sub_solutions] - retained += [cycle.first_state for cycle in sol.cycles] - retained += [cycle.last_state for cycle in sol.cycles] - for solution in retained: - assert vars(solution) == {} + def test_solutions_reject_undefined_attributes(self): + # An experiment keeps several solutions per step, so none carries a __dict__ + t = np.linspace(0, 1) + solution = pybamm.Solution(t, np.tile(t, (2, 1)), pybamm.BaseModel(), {}) + eis_solution = pybamm.EISSolution(np.array([1.0]), np.array([1 + 1j])) + for sol in (solution, solution.first_state, eis_solution): + assert not hasattr(sol, "__dict__") + with pytest.raises(AttributeError, match=r"label"): + sol.label = "extra" @pytest.mark.skipif( sys.version_info < (3, 13), reason="__static_attributes__ is new in 3.13" ) def test_attributes_set_by_methods_are_slots(self): - # An attribute a method sets without a slot would give every solution - # a __dict__ again + # An attribute a method sets without a slot raises AttributeError, which + # a rarely run path could otherwise hide classes = pybamm.Solution.__mro__[:-1] slots = {name for cls in classes for name in cls.__slots__} properties = { @@ -757,15 +754,13 @@ def test_attributes_set_by_methods_are_slots(self): for cls in classes: assert set(cls.__static_attributes__) <= slots | properties - def test_pickle_keeps_slots_and_extra_attributes(self): + def test_pickle_keeps_slots(self): t = np.linspace(0, 1) solution = pybamm.Solution(t, np.tile(t, (2, 1)), pybamm.BaseModel(), {}) first_state = solution.first_state - solution.label = "extra" loaded = pickle.loads(pickle.dumps(solution)) # nosec B301 np.testing.assert_array_equal(loaded.t, t) np.testing.assert_array_equal(loaded.first_state.y, first_state.y) - assert loaded.label == "extra" def test_total_time(self): sol = pybamm.Solution(np.array([0]), np.array([[1, 2]]), pybamm.BaseModel(), {}) @@ -1982,11 +1977,11 @@ def test_statistics_survive_pickling(self): solution = self._solution(0, statistics) assert pickle.loads(pickle.dumps(solution)).solver_statistics == statistics # nosec B301 - def test_solution_pickled_before_slots_loads(self): + def test_solution_pickled_before_slots_loads(self, caplog): solution = self._solution(0, pybamm.SolverStatistics(1, 2, 3, 4, 5)) first_state = solution.first_state # A solution pickled before Solution had slots holds one dict, without - # solver_statistics and with the cached first state under its name + # solver_statistics, with the cached first state under its name state = { name: getattr(solution, name) for cls in type(solution).__mro__ @@ -1995,12 +1990,17 @@ def test_solution_pickled_before_slots_loads(self): } del state["solver_statistics"], state["_first_state"], state["_last_state"] state["first_state"] = first_state + # and perhaps with an attribute a caller attached + state["label"] = "extra" class PreSlotsPickle: def __reduce__(self): return object.__new__, (pybamm.Solution,), state - loaded = pickle.loads(pickle.dumps(PreSlotsPickle())) # nosec B301 + with caplog.at_level(logging.WARNING, logger="pybamm.logger"): + loaded = pickle.loads(pickle.dumps(PreSlotsPickle())) # nosec B301 + assert "Dropped attributes ['label']" in caplog.text + assert not hasattr(loaded, "label") assert loaded.solver_statistics is None np.testing.assert_array_equal(loaded.t, solution.t) np.testing.assert_array_equal(loaded.first_state.y, first_state.y)