diff --git a/CHANGELOG.md b/CHANGELOG.md index 289feb9890..e888767154 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. ([#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)) 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 # ======================================================================