diff --git a/scripts/test.sh b/scripts/test.sh index 377902ba..65d14158 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -46,6 +46,8 @@ if [ $PARALLEL_ONLY -eq 1 ] && [ $PARALLEL_RANKS -eq 0 ]; then fi export UW_NO_USAGE_METRICS=0 +# A hard crash must print a Python stack, not just "Segmentation fault". +export PYTHONFAULTHANDLER=1 PYTEST="pytest --config-file=tests/pytest.ini" # Run serial tests (unless --parallel-only specified) diff --git a/src/underworld3/discretisation/discretisation_mesh.py b/src/underworld3/discretisation/discretisation_mesh.py index 50262af9..750766b4 100644 --- a/src/underworld3/discretisation/discretisation_mesh.py +++ b/src/underworld3/discretisation/discretisation_mesh.py @@ -6816,11 +6816,38 @@ def label_interface_band(self, surface, offset=0.0, halo=1, name=None): if vS <= p < vE]) for c in range(cS, cE)] + def _sync_across_ranks(pinned_set): + """A vertex pinned on ANY rank is pinned on EVERY rank holding a copy. + + The straddle test and the ring growth both walk rank-LOCAL cells, and + cells are partitioned disjointly — so a shared vertex whose cut (or + ring) cell lives on the neighbour rank is pinned there but not here. + If HERE is the owner, the mover moves it and the neighbour's pinned + copy follows through the SF: measured at np=4 (review of PR #488, + 2026-08-06), two pinned leaves moved 4.2e-3 and 1.9e-3 while np=2/3 + passed on partition luck. Matching by coordinate (the same rounded + key the parallel test uses) makes the set partition-independent. + """ + if uw.mpi.size == 1: + return pinned_set + local_xy = (coords[[v - vS for v in pinned_set]] + if pinned_set else numpy.empty((0, self.dim))) + global_keys = set() + for arr in uw.mpi.comm.allgather(local_xy): + for p in arr: + global_keys.add(tuple(numpy.round(p, 12))) + out = set(pinned_set) + for i in range(vE - vS): + if tuple(numpy.round(coords[i], 12)) in global_keys: + out.add(vS + i) + return out + pinned = set() for verts in cell_vertices: d = distance[verts - vS] if d.min() < offset < d.max(): pinned.update(int(v) for v in verts) + pinned = _sync_across_ranks(pinned) for _ring in range(halo): grown = set() for verts in cell_vertices: @@ -6828,10 +6855,17 @@ def label_interface_band(self, surface, offset=0.0, halo=1, name=None): if any(v in pinned for v in vv): grown.update(vv) pinned |= grown - - if not pinned: - # An empty DMLabel is not merely useless: querying its strata is a - # hard crash, not an exception, so refuse rather than hand one back. + pinned = _sync_across_ranks(pinned) + + # COLLECTIVE emptiness test. A rank whose subdomain the surface never + # enters legitimately has an empty local band — only a GLOBALLY empty + # band is a user error. The previous rank-local raise here deadlocked + # np=4 (measured 2026-08-06, review of PR #488: a corner-confined + # surface left three ranks raising while the fourth entered the + # collective mover and hung to the 300 s timeout). + n_global = uw.mpi.comm.allreduce(len(pinned)) + if n_global == 0: + # Raised on EVERY rank, after the collective reduction. raise ValueError( f"no cell is cut by distance == {offset} on surface " f"{getattr(surface, 'name', surface)!r}, so there is no band to " @@ -6839,6 +6873,11 @@ def label_interface_band(self, surface, offset=0.0, halo=1, name=None): f"interface you meant (for a weak zone it is the HALF-WIDTH, not " f"zero).") + # Every rank creates the label, including ranks whose local band is + # empty: the downstream consumer (smoothing.graph._pinned_mask) is + # documented to tolerate a present-but-empty label, and a label that + # exists on some ranks only is the kind of asymmetry this method is + # not allowed to produce. name = name or f"PinnedBand_{getattr(surface, 'name', 'surface')}" if not dm.hasLabel(name): dm.createLabel(name) diff --git a/src/underworld3/utilities/custom_mg.py b/src/underworld3/utilities/custom_mg.py index 9719e6e0..f9d02b81 100644 --- a/src/underworld3/utilities/custom_mg.py +++ b/src/underworld3/utilities/custom_mg.py @@ -1040,6 +1040,24 @@ def build_transfers(solver, field_id=None): if coarse is None: return None, None # nothing to inject + # An EXPLICIT preconditioner choice beats the opportunistic pickup. Before + # this guard, `solver.preconditioner = "gamg"` on an adapt child was + # silently clobbered back to the custom-P PCMG at solve time (measured: + # both arms of test_0842's fmg-vs-gamg comparison ran pc_type=mg), so a + # user could not opt out and any FMG-vs-GAMG comparison was vacuous. + # `_pc_user_override` is the same statement in the other spelling: the + # solver's option manager has latched "the user owns this block's pc_type" + # (they wrote a pc_type of their own into petsc_options), and an + # opportunistic pickup must stand down for exactly the same reason. + # "auto" (the default) still picks up the mesh-owned hierarchy. + # NOTE the arity: this function returns a 2-tuple, never bare None — a bare + # `return` here is what turned the gate into a TypeError at the call site + # when this hunk migrated from auto_inject_custom_mg (which returns nothing) + # during the #488 x #471 merge. + if (getattr(solver, "_preconditioner", "auto") == "gamg" + or getattr(solver, "_pc_user_override", False)): + return None, None + builder = getattr(solver.mesh, "_custom_mg_builder", "barycentric") # Retry with the RBF builder before abandoning geometric MG. The # barycentric builder has LOCAL support: it re-triangulates the coarse @@ -1098,8 +1116,13 @@ def auto_inject_custom_mg(solver, field_id=None): inject_custom_mg(solver) return - h, Ps = build_transfers(solver, field_id=field_id) - if h is None: + # build_transfers' contract is a 2-tuple, but a "no hierarchy" answer has + # been written as a bare `return` before (the #488 x #471 merge shipped + # exactly that inside the explicit-gamg gate): a None here must mean + # "nothing to inject", never a TypeError mid-solve. + resolved = build_transfers(solver, field_id=field_id) + h, Ps = resolved if resolved is not None else (None, None) + if h is None or Ps is None: return # Dimensional guard (checkable for the monolithic operator, field_id is None): diff --git a/src/underworld3/utilities/rotated_bc.py b/src/underworld3/utilities/rotated_bc.py index 3d521845..7d00a499 100644 --- a/src/underworld3/utilities/rotated_bc.py +++ b/src/underworld3/utilities/rotated_bc.py @@ -907,8 +907,11 @@ def _build_rotated_custom_Pl(solver, Q, normal_rows): free-slip silently lost its multigrid and solved on GAMG (#467). This is the ``adapt-on-top-faults`` workflow's own configuration.""" from underworld3.utilities import custom_mg - h, Ps = custom_mg.build_transfers(solver, field_id=0) - if h is None: + # Same None discipline as auto_inject_custom_mg: the contract is a 2-tuple, + # but "no hierarchy" must degrade to the default preconditioner, never raise. + resolved = custom_mg.build_transfers(solver, field_id=0) + h, Ps = resolved if resolved is not None else (None, None) + if h is None or Ps is None: return None vel_is = solver._subdict["velocity"][0] vis = np.asarray(vel_is.getIndices()) diff --git a/tests/parallel/ptest_0845_relax_pinned_band_parallel.py b/tests/parallel/ptest_0845_relax_pinned_band_parallel.py index 7461c4db..ed37c053 100644 --- a/tests/parallel/ptest_0845_relax_pinned_band_parallel.py +++ b/tests/parallel/ptest_0845_relax_pinned_band_parallel.py @@ -47,7 +47,15 @@ def _coords(mesh): def _pinned_indices(mesh, name): vS, _vE = mesh.dm.getDepthStratum(0) - iset = mesh.dm.getLabel(name).getStratumIS(1) + label = mesh.dm.getLabel(name) + # A rank the surface never enters has the label but NO strata, and + # getStratumIS on an empty DMLabel is a segfault, not an exception + # (#291) — the same tolerant pattern smoothing.graph._pinned_mask uses. + # At np=4 this fixture leaves one rank band-less, which is exactly the + # partition case the 2026-08-02 review found uncovered. + if label.getNumValues() == 0: + return np.zeros(0, dtype=np.int64) + iset = label.getStratumIS(1) if iset is None: return np.zeros(0, dtype=np.int64) return np.asarray(iset.getIndices(), dtype=np.int64) - vS @@ -93,7 +101,10 @@ def test_pinned_vertices_including_shared_ones_do_not_move(): after = _coords(mesh) moved = np.linalg.norm(after - before, axis=1) - assert uw.mpi.comm.allreduce(float(moved[idx].max()), op=MPI.MAX) == 0.0 + # A band-less rank (np=4 leaves one) has empty idx; max() of a zero-size + # array raises rank-locally and desyncs the collectives below. + assert uw.mpi.comm.allreduce( + float(moved[idx].max()) if len(idx) else 0.0, op=MPI.MAX) == 0.0 free = np.setdiff1d(np.arange(len(before)), idx) assert uw.mpi.comm.allreduce(float(moved[free].max()) if len(free) else 0.0, op=MPI.MAX) > 0.0, "the mover did nothing" diff --git a/tests/test_0842_nvb_3d_parallel_adapt.py b/tests/test_0842_nvb_3d_parallel_adapt.py index fe95708d..4fd05a2d 100644 --- a/tests/test_0842_nvb_3d_parallel_adapt.py +++ b/tests/test_0842_nvb_3d_parallel_adapt.py @@ -90,8 +90,15 @@ def test_poisson_fmg_on_3d_child_matches_gamg(): mesh = _base3() child = mesh.adapt(_ball_metric, max_levels=1) + # Deliberate ordering: create BOTH variables before any solver runs — + # creating a MeshVariable after a solve rebuilds mesh.dm and detonates + # issue #492 (the old DM is destroyed under the custom-MG coarse/fine + # links; that dangling reference is what segfaulted Linux CI downstream). + fields = {pc: uw.discretisation.MeshVariable(f"u_{pc}", child, 1, degree=1) + for pc in ("fmg", "gamg")} + def solve(pc): - u = uw.discretisation.MeshVariable(f"u_{pc}", child, 1, degree=1) + u = fields[pc] poisson = uw.systems.Poisson(child, u_Field=u) poisson.constitutive_model = uw.constitutive_models.DiffusionModel poisson.constitutive_model.Parameters.diffusivity = 1.0 @@ -101,15 +108,37 @@ def solve(pc): if pc == "gamg": poisson.preconditioner = "gamg" poisson.petsc_options["pc_type"] = "gamg" - poisson.petsc_options["ksp_rtol"] = 1e-8 + poisson.petsc_options["ksp_rtol"] = 1e-9 poisson.solve() - its = poisson.snes.getKSP().getIterationNumber() + ksp = poisson.snes.getKSP() + its = ksp.getIterationNumber() + # The comparison must be REAL. The explicit-gamg arm was once silently + # clobbered by the mesh-owned custom-P pickup, so both arms ran + # pc_type=mg and the fmg-vs-gamg comparison compared FMG to itself. + # Pin each arm's PC so that vacuous comparison can never return. + assert ksp.getPC().getType() == ("gamg" if pc == "gamg" else "mg") # exact linear solution T = z: also proves the Dirichlet facet - # labels survived the parallel transform + # labels survived the parallel transform. + # + # The bound is tight ON PURPOSE: it must catch a once-shipped defect + # whose signature was a TRUE-error stall at 1e-6, INSENSITIVE to + # ksp_rtol — the gmres-smoothed geometric bundle (a non-stationary + # preconditioner) under a plain left-preconditioned gmres outer, whose + # recurrence norm fell to 1e-11 while the true residual stalled + # (the geometric bundle now owns the pairing — multigrid_options puts + # ksp_type=fgmres in the bundle and _configure_pcmg applies it to the + # live KSP, #514/#515 — so the fmg arm's ksp_rtol is enforced in the + # true residual norm; + # measured err/nrm ~1e-12). The gamg arm still converges in the + # preconditioned norm, with a declared-reduction -> nodal-error + # constant of ~10 on this child, so the declared reduction is one + # order tighter than the bound: neither arm rides on its PC constant, + # and the O(1) failures this test exists for (a lost Dirichlet label, + # a wrong transfer) stay unmissable. err = np.linalg.norm( poisson.Unknowns.u.data[:, 0] - poisson.Unknowns.u.coords[:, 2]) nrm = np.linalg.norm(poisson.Unknowns.u.coords[:, 2]) + 1e-30 - assert err / nrm < 1e-8 + assert err / nrm < 1e-7 return its fmg_its = solve("fmg")