From c774ba520c77543e4748f45eac72fd31c3660fb5 Mon Sep 17 00:00:00 2001 From: Alexander Bills Date: Thu, 6 Aug 2026 17:18:23 -0700 Subject: [PATCH 1/2] fix: invalidate the tagged-mesh cache when the source file changes TaggedSubMeshGenerator cached parsed .msh files in an unbounded, class-level dict keyed on the raw path object. An edited file was never re-read (a stale mesh was returned silently), str and pathlib.Path paths double-cached, and the cache grew without bound. Replace it with functools.lru_cache keyed on (os.fspath(path), st_mtime_ns): editing the file invalidates the entry, str/Path collapse to one key, and the cache is size-bounded. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + .../src/pybamm/meshes/unstructured_submesh.py | 18 ++- .../test_meshes/test_unstructured_submesh.py | 117 +++++++++++------- 3 files changed, 84 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 289feb9890..e4a2485f5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ## Bug fixes +- `TaggedSubMeshGenerator` no longer returns a stale cached mesh when the source `.msh` file changes on disk: its mesh cache is keyed on the file's modification time, collapses `str`/`pathlib.Path` paths to one entry, and is bounded in size. ([#XXXX](https://github.com/pybamm-team/PyBaMM/pull/XXXX)) - `ElectrodeSOHSolver` now passes model options through, so hysteresis OCP branches are used. ([#5701](https://github.com/pybamm-team/PyBaMM/pull/5701)) - Fixed a memory leak in `ElectrodeSOHSolver.theoretical_energy_integral`, which cached a new expression tree per call. ([#5695](https://github.com/pybamm-team/PyBaMM/pull/5695)) - `BatchStudy.solve` no longer ignores its `solver` argument: previously the loop over study inputs shadowed it, so a caller-supplied solver was silently dropped. A solver from `BatchStudy(solvers=...)` still takes precedence. ([#5677](https://github.com/pybamm-team/PyBaMM/pull/5677)) diff --git a/packages/pybamm/src/pybamm/meshes/unstructured_submesh.py b/packages/pybamm/src/pybamm/meshes/unstructured_submesh.py index f2d344fd44..5e8ca78a66 100644 --- a/packages/pybamm/src/pybamm/meshes/unstructured_submesh.py +++ b/packages/pybamm/src/pybamm/meshes/unstructured_submesh.py @@ -1,3 +1,5 @@ +import functools +import os from enum import Enum import numpy as np @@ -1048,8 +1050,6 @@ class TaggedSubMeshGenerator(MeshGenerator): it the submesh carries no boundary tags. """ - _mesh_cache: dict = {} - def __init__( self, region, mesh_path, scale=1.0, coord_sys="cartesian", boundary_mapping=None ): @@ -1061,12 +1061,18 @@ def __init__( self.coord_sys = coord_sys self.boundary_mapping = boundary_mapping or {} + @staticmethod + @functools.lru_cache(maxsize=8) + def _read_cached(fspath, mtime_ns): + meshio = pybamm.import_optional_dependency("meshio") + return meshio.read(fspath) + @classmethod def _read(cls, path): - if path not in cls._mesh_cache: - meshio = pybamm.import_optional_dependency("meshio") - cls._mesh_cache[path] = meshio.read(str(path)) - return cls._mesh_cache[path] + # Key on (path, mtime) so an edited file is re-read; os.fspath collapses + # str/Path to one entry and lru_cache bounds the retained meshes. + fspath = os.fspath(path) + return cls._read_cached(fspath, os.stat(fspath).st_mtime_ns) def __call__(self, lims, npts): m = self._read(self._mesh_path) diff --git a/packages/pybamm/tests/unit/test_meshes/test_unstructured_submesh.py b/packages/pybamm/tests/unit/test_meshes/test_unstructured_submesh.py index f37772781d..d112b2a858 100644 --- a/packages/pybamm/tests/unit/test_meshes/test_unstructured_submesh.py +++ b/packages/pybamm/tests/unit/test_meshes/test_unstructured_submesh.py @@ -770,7 +770,7 @@ def test_user_supplied_boundary_mapping_tags_faces(self): np.testing.assert_allclose(sorted(map(tuple, seal)), [(0.5, 0.0), (1.0, 0.5)]) np.testing.assert_allclose(sorted(map(tuple, vent)), [(0.0, 0.5), (0.5, 1.0)]) - def test_tagged_generator_boundary_mapping(self): + def test_tagged_generator_boundary_mapping(self, monkeypatch): """TaggedSubMeshGenerator resolves surface physical groups to tags.""" import pytest @@ -784,19 +784,17 @@ def test_tagged_generator_boundary_mapping(self): cell_data={"gmsh:physical": [np.full(5, 1), np.full(2, 10)]}, field_data={"anode": np.array([1, 3]), "base": np.array([10, 2])}, ) - fake_path = "fake_tagged_boundaries.msh" - TaggedSubMeshGenerator._mesh_cache[fake_path] = mesh - try: - gen = TaggedSubMeshGenerator( - "anode", fake_path, boundary_mapping={"bottom_seal": "base"} - ) - sub = gen({}, {}) - assert set(sub.boundary_faces) == {"bottom_seal"} - centroids = sub.face_centroids[sub.boundary_faces["bottom_seal"]] - np.testing.assert_allclose(centroids[:, 2], 0.0, atol=1e-14) - assert len(centroids) == 2 - finally: - TaggedSubMeshGenerator._mesh_cache.pop(fake_path, None) + monkeypatch.setattr( + TaggedSubMeshGenerator, "_read", classmethod(lambda cls, path: mesh) + ) + gen = TaggedSubMeshGenerator( + "anode", "unused.msh", boundary_mapping={"bottom_seal": "base"} + ) + sub = gen({}, {}) + assert set(sub.boundary_faces) == {"bottom_seal"} + centroids = sub.face_centroids[sub.boundary_faces["bottom_seal"]] + np.testing.assert_allclose(centroids[:, 2], 0.0, atol=1e-14) + assert len(centroids) == 2 def test_user_supplied_no_supported_cells_raises(self): """A mesh with only unsupported cell types raises.""" @@ -866,50 +864,77 @@ def _tagged_gmsh_mesh(): field_data={"anode": np.array([1, 3]), "cathode": np.array([2, 3])}, ) - def test_tagged_generator_extracts_region(self): + def test_tagged_generator_extracts_region(self, monkeypatch): import pytest pytest.importorskip("meshio") - fake_path = "fake_tagged_mesh.msh" - TaggedSubMeshGenerator._mesh_cache[fake_path] = self._tagged_gmsh_mesh() - try: - gen = TaggedSubMeshGenerator("anode", fake_path, scale=2.0) - sub = gen({}, {}) - assert isinstance(sub, UnstructuredSubMesh) - assert sub.npts == 3 # cells tagged 1 - # scale multiplies coordinates: unit cube -> side 2 - np.testing.assert_allclose(sub.vertices.max(axis=0), [2.0, 2.0, 2.0]) - finally: - TaggedSubMeshGenerator._mesh_cache.pop(fake_path, None) - - def test_tagged_generator_missing_region_raises(self): + monkeypatch.setattr( + TaggedSubMeshGenerator, + "_read", + classmethod(lambda cls, path: self._tagged_gmsh_mesh()), + ) + gen = TaggedSubMeshGenerator("anode", "unused.msh", scale=2.0) + sub = gen({}, {}) + assert isinstance(sub, UnstructuredSubMesh) + assert sub.npts == 3 # cells tagged 1 + # scale multiplies coordinates: unit cube -> side 2 + np.testing.assert_allclose(sub.vertices.max(axis=0), [2.0, 2.0, 2.0]) + + def test_tagged_generator_missing_region_raises(self, monkeypatch): import pytest pytest.importorskip("meshio") - fake_path = "fake_tagged_mesh_2.msh" - TaggedSubMeshGenerator._mesh_cache[fake_path] = self._tagged_gmsh_mesh() - try: - gen = TaggedSubMeshGenerator("does-not-exist", fake_path) - with pytest.raises(pybamm.GeometryError, match="not in mesh field_data"): - gen({}, {}) - finally: - TaggedSubMeshGenerator._mesh_cache.pop(fake_path, None) - - def test_tagged_generator_region_without_tets_raises(self): + monkeypatch.setattr( + TaggedSubMeshGenerator, + "_read", + classmethod(lambda cls, path: self._tagged_gmsh_mesh()), + ) + gen = TaggedSubMeshGenerator("does-not-exist", "unused.msh") + with pytest.raises(pybamm.GeometryError, match="not in mesh field_data"): + gen({}, {}) + + def test_tagged_generator_region_without_tets_raises(self, monkeypatch): import pytest pytest.importorskip("meshio") - fake_path = "fake_tagged_mesh_3.msh" mesh = self._tagged_gmsh_mesh() # Physical group 9 exists in field_data but tags no tet cells mesh.field_data["empty"] = np.array([9, 3]) - TaggedSubMeshGenerator._mesh_cache[fake_path] = mesh - try: - gen = TaggedSubMeshGenerator("empty", fake_path) - with pytest.raises(pybamm.GeometryError, match="no tets"): - gen({}, {}) - finally: - TaggedSubMeshGenerator._mesh_cache.pop(fake_path, None) + monkeypatch.setattr( + TaggedSubMeshGenerator, "_read", classmethod(lambda cls, path: mesh) + ) + gen = TaggedSubMeshGenerator("empty", "unused.msh") + with pytest.raises(pybamm.GeometryError, match="no tets"): + gen({}, {}) + + def test_tagged_generator_cache_reads_and_invalidates(self, tmp_path): + import os + + import pytest + + meshio = pytest.importorskip("meshio") + TaggedSubMeshGenerator._read_cached.cache_clear() + + path = tmp_path / "cache_demo.msh" + meshio.write( + str(path), + meshio.Mesh( + np.array([[0, 0, 0], [1, 0, 0], [0, 1, 0], [0, 0, 1]], dtype=float), + [("tetra", np.array([[0, 1, 2, 3]]))], + ), + file_format="gmsh22", + binary=False, + ) + + first = TaggedSubMeshGenerator._read(path) + # unchanged file: cache hit returns the same object + assert TaggedSubMeshGenerator._read(path) is first + # str and pathlib.Path collapse to a single cache entry + assert TaggedSubMeshGenerator._read(str(path)) is first + # a newer modification time invalidates the entry and re-reads + stat = os.stat(path) + os.utime(path, ns=(stat.st_atime_ns, stat.st_mtime_ns + 1_000_000_000)) + assert TaggedSubMeshGenerator._read(path) is not first # ====================================================================== From 6957fadf028b7d1bc53bdeefcf0e7bbacb16ce1a Mon Sep 17 00:00:00 2001 From: Alexander Bills Date: Thu, 6 Aug 2026 17:18:54 -0700 Subject: [PATCH 2/2] docs: changelog link for the tagged-mesh cache fix Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4a2485f5a..e888767154 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ ## Bug fixes -- `TaggedSubMeshGenerator` no longer returns a stale cached mesh when the source `.msh` file changes on disk: its mesh cache is keyed on the file's modification time, collapses `str`/`pathlib.Path` paths to one entry, and is bounded in size. ([#XXXX](https://github.com/pybamm-team/PyBaMM/pull/XXXX)) +- `TaggedSubMeshGenerator` no longer returns a stale cached mesh when the source `.msh` file changes on disk: its mesh cache is keyed on the file's modification time, collapses `str`/`pathlib.Path` paths to one entry, and is bounded in size. ([#5705](https://github.com/pybamm-team/PyBaMM/pull/5705)) - `ElectrodeSOHSolver` now passes model options through, so hysteresis OCP branches are used. ([#5701](https://github.com/pybamm-team/PyBaMM/pull/5701)) - Fixed a memory leak in `ElectrodeSOHSolver.theoretical_energy_integral`, which cached a new expression tree per call. ([#5695](https://github.com/pybamm-team/PyBaMM/pull/5695)) - `BatchStudy.solve` no longer ignores its `solver` argument: previously the loop over study inputs shadowed it, so a caller-supplied solver was silently dropped. A solver from `BatchStudy(solvers=...)` still takes precedence. ([#5677](https://github.com/pybamm-team/PyBaMM/pull/5677))