From a3acd33f9ca16b238375f0369aba3a6833750189 Mon Sep 17 00:00:00 2001 From: lmoresi Date: Wed, 29 Apr 2026 16:01:49 +1000 Subject: [PATCH] Fix SNES + DM-hierarchy leak in _build() rebuild path (#157) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The full-rebuild path of SolverBaseClass._build() (the path taken on every is_setup=False) was leaking two PETSc objects: (1) self.snes — _setup_solver() always does self.snes = PETSc.SNES().create(...) The previous SNES (with its KSP, PC, and full GAMG hierarchy of coarse operator matrices) was just overwritten, never destroyed. (2) self.dm_hierarchy[:-1] — _build() only called self.dm.destroy(), which is dm_hierarchy[-1]. All coarser levels in the hierarchy from the previous build leaked. _reset() already had the correct destroy sequence (lines 280-294); _build() just didn't borrow from it. Tight is_setup=False loops compound this. SNES_Tensor_Projection.solve() cycles 6 components in 3D (3 in 2D), setting is_setup=False between each, so a single tensor projection accumulates one leaked SNES + one leaked coarse-DM hierarchy per component. At Gadi scale (issue #157 reports 1.92 TB used / 1440 ranks during stress recovery before being killed), the SNES bag — each carrying its full GAMG coarse operator stack — is plausibly the dominant contributor. Fix: bring _build()'s teardown into line with _reset() — destroy SNES first, then iterate dm_hierarchy destroying each coarse DM, before reconstructing. self.snes / self.dm_hierarchy may not exist on the first build, so the SNES guard uses getattr-style checking. Verified: pytest -m "level_1 and tier_a" on amr-dev passes 56/3/0, no regressions in any of the existing solver test paths. Note: this complements but does not replace SNES_MultiComponent_Projection (PR #124) for the symmetric-tensor stress recovery use case in #157; that path collapses N components into one SNES solve and is the preferred architectural answer. This fix plugs the leak in the existing per-component path so users of SNES_Tensor_Projection don't silently accumulate memory while we migrate to the new path. Underworld development team with AI support from Claude Code --- .../cython/petsc_generic_snes_solvers.pyx | 40 +++++++++++++++++-- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/src/underworld3/cython/petsc_generic_snes_solvers.pyx b/src/underworld3/cython/petsc_generic_snes_solvers.pyx index e5c483e3b..1aa27dc50 100644 --- a/src/underworld3/cython/petsc_generic_snes_solvers.pyx +++ b/src/underworld3/cython/petsc_generic_snes_solvers.pyx @@ -596,12 +596,44 @@ class SolverBaseClass(uw_object): self._last_jit_cache_key = self._current_jit_cache_key return - # === Full rebuild path — teardown DM and reconstruct === - if self.dm is not None: + # === Full rebuild path — teardown DM/SNES and reconstruct === + # BUGFIX(#157): two PETSc objects were leaking on every is_setup=False + # rebuild: + # (1) self.snes — _setup_solver() does + # self.snes = PETSc.SNES().create(...) + # unconditionally; the previous SNES (with its KSP, PC, and + # full GAMG hierarchy of coarse operator matrices) was just + # overwritten. + # (2) self.dm_hierarchy[:-1] — only self.dm == dm_hierarchy[-1] + # was destroyed here; coarse DMs from the previous build + # leaked. + # Tight is_setup=False loops (notably SNES_Tensor_Projection.solve() + # cycling 6 symmetric tensor components in 3D) accumulate one SNES + # plus one full coarse-DM hierarchy per iteration — large enough at + # Gadi scale to push past memory limits during stress recovery + # postprocessing. _reset() already had the correct destroy + # sequence; this brings _build() into line with it. + # NB self.snes / self.dm_hierarchy may not exist yet on the first + # build, so use getattr/hasattr-style guards rather than `is not None`. + if getattr(self, "snes", None) is not None: if verbose and uw.mpi.rank == 0: - print(f"Destroy solver DM", flush=True) + print(f"Destroy solver SNES", flush=True) + self.snes.destroy() + self.snes = None - self.dm.destroy() + if self.dm is not None: + if verbose and uw.mpi.rank == 0: + print(f"Destroy solver DM hierarchy", flush=True) + + if hasattr(self, "dm_hierarchy") and self.dm_hierarchy: + # Destroys each level — including dm_hierarchy[-1] which + # is self.dm — so no separate self.dm.destroy() needed. + for coarse_dm in self.dm_hierarchy: + if coarse_dm is not None: + coarse_dm.destroy() + self.dm_hierarchy = [None] + else: + self.dm.destroy() self.dm = None if hasattr(self, "_stokes_nullspace"): self._stokes_nullspace = None