Skip to content

Commit 7b9fdc7

Browse files
committed
Address Copilot review on #136
- _get_coords_for_basis: destroy matInterp/vecScale after use so the per-variable eager cache fill and the nuke_coords_and_rebuild refill loop don't leak PETSc objects in long adapt runs. - test_0780_ve_stokes_first_solve_mpi: require mpi(min_size=4). The bug only reproduces at the 4x2 partition geometry; min_size=2 would let CI pass with a partition that never hit the deadlock. - Relabel the two bugfix comment blocks from TODO(BUG) to BUGFIX(#130) so they don't read as unfinished work. Underworld development team with AI support from Claude Code
1 parent fd9b039 commit 7b9fdc7

3 files changed

Lines changed: 14 additions & 8 deletions

File tree

src/underworld3/discretisation/discretisation_mesh.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1653,10 +1653,10 @@ def nuke_coords_and_rebuild(
16531653
# Invalidate projected boundary normals (rebuilt lazily on access)
16541654
self._projected_normals = None
16551655

1656-
# TODO(BUG): issue #130 — refill the coord cache for every already-
1657-
# registered variable. Variables created before this rebuild would
1658-
# otherwise have their cache entry (from __init__) wiped above and
1659-
# refill lazily from rank-local code paths (rbf_interpolate), which
1656+
# BUGFIX(#130): refill the coord cache for every already-registered
1657+
# variable. Variables created before this rebuild would otherwise
1658+
# have their cache entry (from __init__) wiped above and refill
1659+
# lazily from rank-local code paths (rbf_interpolate), which
16601660
# deadlocks when the collectives inside _get_coords_for_basis are
16611661
# reached by only a subset of ranks.
16621662
for _var in list(self.vars.values()):
@@ -2733,6 +2733,12 @@ def _get_coords_for_basis(self, degree, continuous):
27332733

27342734
dmnew.restoreGlobalVec(coordsNewG)
27352735
dmnew.restoreLocalVec(coordsNewL)
2736+
# Clean up the PETSc interpolation objects built above. Without this
2737+
# they accumulate until Python GC runs — noticeable in long adapt
2738+
# loops that re-fill the coord cache per variable.
2739+
matInterp.destroy()
2740+
if vecScale is not None:
2741+
vecScale.destroy()
27362742
dmnew.destroy()
27372743
dmfe.destroy()
27382744

src/underworld3/discretisation/discretisation_mesh_variables.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,8 +388,8 @@ def __init__(
388388
self.mesh.vars[self.clean_name] = self
389389
self._setup_ds()
390390

391-
# TODO(BUG): issue #130 — pre-populate the mesh's coordinate cache
392-
# for this variable's basis. mesh._get_coords_for_basis contains MPI
391+
# BUGFIX(#130): pre-populate the mesh's coordinate cache for this
392+
# variable's basis. mesh._get_coords_for_basis contains MPI
393393
# collectives (DMClone, createInterpolation, globalToLocal) that
394394
# deadlock when triggered lazily from rank-local code paths (e.g.
395395
# rbf_interpolate inside global_evaluate_nd's per-particle loop):

tests/parallel/test_0780_ve_stokes_first_solve_mpi.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@
2727
pytestmark = [
2828
pytest.mark.level_2,
2929
pytest.mark.tier_a,
30-
pytest.mark.mpi(min_size=2),
30+
pytest.mark.mpi(min_size=4),
3131
pytest.mark.timeout(60),
3232
]
3333

3434

35-
@pytest.mark.mpi(min_size=2)
35+
@pytest.mark.mpi(min_size=4)
3636
def test_ve_stokes_first_solve_does_not_deadlock():
3737
"""
3838
First VE_Stokes.solve() must complete under MPI on a partition-sensitive

0 commit comments

Comments
 (0)