From 56d3d37fa9a7794c9a79d51eb3c309773e068e81 Mon Sep 17 00:00:00 2001 From: Louis Moresi Date: Sat, 4 Jul 2026 13:06:18 +1000 Subject: [PATCH 1/2] fix(rotated-bc): warn on non-converged linear KSP `ksp.solve` never raises on `KSP_DIVERGED_*`, so the two linear paths in `rotated_bc` (the default fieldsplit-Schur iterative solve at `_solve_rotated_iterative` and the opt-in direct MUMPS LU at `solve_rotated_freeslip`) both returned partial answers silently. That silence is what let the 3D rotation-nullspace bug fixed by #306 look like a working solve for as long as it did: the outer Krylov hit its `ksp_max_it` ceiling (300 in the pre-#306 code) with a residual well above tolerance, and neither the KSP nor the caller flagged it. The solution then propagated into downstream diagnostics that eventually exposed it as wrong physics rather than as a solver failure. Add a small `_warn_if_ksp_diverged` helper and call it immediately after each of the two `ksp.solve` sites. Rank-0 warning only (via `uw.mpi.pprint`), same style as the existing nonlinear-driver warning at the end of `solve_rotated_freeslip_nonlinear`. No behaviour change on the happy path; non-convergence is now loud instead of silent. test_1018_rotated_freeslip.py: 14/14 pass (2D behaviour unchanged). Underworld development team with AI support from Claude Code --- src/underworld3/utilities/rotated_bc.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/underworld3/utilities/rotated_bc.py b/src/underworld3/utilities/rotated_bc.py index e6afd60f..47278a9c 100644 --- a/src/underworld3/utilities/rotated_bc.py +++ b/src/underworld3/utilities/rotated_bc.py @@ -31,6 +31,27 @@ _ROT_SOLVE_COUNT = 0 +def _warn_if_ksp_diverged(ksp, kind): + """Emit a rank-0 warning if a KSP finished on a KSP_DIVERGED_* reason + (negative converged-reason). ``ksp.solve`` never raises on divergence, so + without this the linear rotated-freeslip solvers here would return a + partial answer to the caller silently — which is exactly what let the 3D + rotation-nullspace bug (#306) go unnoticed until it produced clearly-wrong + physics in a downstream test.""" + reason = int(ksp.getConvergedReason()) + if reason >= 0: + return + its = int(ksp.getIterationNumber()) + try: + rnorm = float(ksp.getResidualNorm()) + except Exception: + rnorm = float("nan") + from underworld3 import mpi + mpi.pprint(f"[rotated_bc] WARNING: {kind} KSP did NOT converge " + f"(reason={reason}, iterations={its}, |r|={rnorm:.3e}); " + f"proceeding with the last iterate.") + + # --------------------------------------------------------------------------- # # Rotation construction # --------------------------------------------------------------------------- # @@ -291,6 +312,7 @@ def solve_rotated_freeslip(solver, boundaries, remove_rotation_gauge=True, verbo pc = ksp.getPC(); pc.setType("lu"); pc.setFactorSolverType("mumps") Uhat = dm.createGlobalVec(); ksp.solve(bhat, Uhat) # returned in info → own it ksp_reason = ksp.getConvergedReason(); ksp_its = ksp.getIterationNumber() + _warn_if_ksp_diverged(ksp, kind="rotated direct-LU") else: Mp = _pressure_mass_schur_pmat(solver) Uhat, ksp_reason, ctx = _solve_rotated_iterative( @@ -797,6 +819,7 @@ def _solve_rotated_iterative(solver, Ahat, bhat, Q, Qt, normal_rows, verbose=Fal Uhat = Ahat.createVecRight(); Uhat.set(0.0) ksp.solve(bhat, Uhat) + _warn_if_ksp_diverged(ksp, kind="rotated fieldsplit-Schur") # An identity constraint row in an ITERATIVE solve only drives its residual # (= û_i) below tolerance, so û_i ~ tol, not exactly 0. Because zeroRowsColumns # made these DOFs fully decoupled (row AND column zeroed), û_i affects no other From 89cf405c4896ba915a8ab4e5869dc90ef64605c9 Mon Sep 17 00:00:00 2001 From: Louis Moresi Date: Sun, 5 Jul 2026 19:44:45 +1000 Subject: [PATCH 2/2] review(Copilot): treat KSP_CONVERGED_ITERATING (reason=0) as non-converged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review on #315 noted the convention split: * KSP.getConvergedReason() > 0 → converged * KSP.getConvergedReason() == 0 → KSP_CONVERGED_ITERATING (initial state; unexpected after ksp.solve() returns) * KSP.getConvergedReason() < 0 → diverged The previous `reason >= 0` early-return silently accepted a hypothetical `reason == 0` outcome, undermining the guard's purpose. Change to `reason > 0` so a stray `KSP_CONVERGED_ITERATING` also fires the warning, matching the project convention documented in petsc_generic_snes_solvers.pyx:2979. test_1018_rotated_freeslip.py: 14/14 pass (unchanged). Underworld development team with AI support from Claude Code --- src/underworld3/utilities/rotated_bc.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/underworld3/utilities/rotated_bc.py b/src/underworld3/utilities/rotated_bc.py index 47278a9c..8ccc1699 100644 --- a/src/underworld3/utilities/rotated_bc.py +++ b/src/underworld3/utilities/rotated_bc.py @@ -39,7 +39,10 @@ def _warn_if_ksp_diverged(ksp, kind): rotation-nullspace bug (#306) go unnoticed until it produced clearly-wrong physics in a downstream test.""" reason = int(ksp.getConvergedReason()) - if reason >= 0: + # Convention: > 0 == converged, < 0 == diverged, 0 == KSP_CONVERGED_ITERATING + # (should not happen after ksp.solve() returns; warn if it does). Matches + # petsc_generic_snes_solvers.pyx:2979. + if reason > 0: return its = int(ksp.getIterationNumber()) try: