Ask an un-set-up field split for its blocks quietly (#827)

docs/reviews/2026-10/subksp-banner-827-adversarial-review.md

@@ -0,0 +1,70 @@
+# Quiet sub-solver query (#827): adversarial review record
+
+Branch `bugfix/subksp-banner-827`, off `development`. One independent reviewer,
+run against the installed build of the branch, with an older feature branch
+carrying the same unguarded call sites as the unfixed reference.
+
+## What the branch does
+
+`_fieldsplit_sub_ksps(pc)` (`utilities/_petsc_tools.py`) returns a field split's
+block solvers, or `[]` when the split is not set up, with PETSc's error banner
+silenced for that one call (the `"return"` handler: the error is still raised, not
+printed; popped on every path). Two call sites use it:
+
+| Site | Path that printed the banner |
+|---|---|
+| `SNES_Stokes._check_velocity_preconditioner` | the viscoelastic solve ran the check before solving (the #827 reproducer); a plain warm solve that converged without a linear solve after a `consistent_jacobian` change rebuilt the solver |
+| `custom_mg._pcmg_still_installed` | the pre-solve "still installed?" test on a solver rebuilt by a `consistent_jacobian` change; every notch run with a non-nested FMG tail printed it |
+
+The viscoelastic check now runs after the solve, as on the plain path.
+
+## Findings and their state
+
+- **The plain-path site had no test** (medium). A loud helper in `systems.solvers`
+  passed every test. Fixed: `test_warm_solve_without_a_linear_solve_prints_no_banner`;
+  12 banner lines unfixed, 0 fixed.
+- **Viscoelastic solves now warn and record a fallback** (behaviour change, kept).
+  Before, the pre-solve check always returned early, so a viscoelastic solve on a
+  mesh without a hierarchy never reported running GAMG. It now warns (once per
+  process under the default filter) and records `velocity.fell_back_from_fmg`, as a
+  plain solve does. Separately and pre-existing: `stacklevel=3` resolves to
+  `memprobe.py`, so every solve shares one warning location.
+- **The helper reached the `uw` namespace** through `from .utilities._petsc_tools
+  import *`. Fixed: renamed `_fieldsplit_sub_ksps`.
+- **Second copy of the pattern** in `petsc_generic_snes_solvers.pyx`
+  (`_discard_preconditioner_setup`). Left: the solver core is not changed in a
+  bugfix; a follow-up can route it through the helper.
+- **Docstring** told the debugging history and lacked Parameters/Returns. Fixed.
+
+## Attacks that failed
+
+- Handler semantics: on an un-set-up Schur split the helper returns `[]` silently;
+  a direct call afterwards still raises and prints, so the stack is restored; a
+  `KeyboardInterrupt` inside propagates with the stack restored; 1000 calls leave it
+  intact; after set-up it returns 2 blocks.
+- Parallel (np 2 and 4): helper before/after set-up, the viscoelastic reproducer,
+  3 viscoelastic steps, the custom-FMG rewire. Exit 0, no banner, no hang. Set-up
+  state follows `PCSetUp`, which every rank runs; the not-set-up error is a local
+  check, so no rank takes a different branch.
+- `_pcmg_still_installed` returns `False` for an un-set-up split as before (it used
+  to raise and be caught); no extra re-installs.
+- Other `getFieldSplitSubKSP` calls (`custom_mg.py` transfer install,
+  `rotated_bc.py`, `solver_health.py`, the solve-config dump) all follow a
+  `setUp`, run inside a solve, or are already guarded.
+- No import cycle: `_petsc_tools` imports only petsc4py.
+- Tests: all three regression tests fail on the unfixed code on the banner
+  assertion and pass fixed; `test_107*` is in `scripts/test.sh`;
+  `check_test_coverage.py` reports every file reachable.
+
+## Suites
+
+Related suites on the fixed build (multigrid, custom-MG hierarchy, rotated
+free-slip, MG option bundle, restore/continuation, viscoelastic): 77 passed, no
+banner. The reviewer's wider run (388 tests, `not tier_c`): 387 passed, 1 failed:
+`test_0017_describe_and_render.py::test_every_family_describes_itself_at_the_class_level`,
+order-dependent and unrelated. `test_1057_ddt_transport_plugin.py` defines a
+solver subclass `TensorTransport` with no summary, and the class-level description
+test enumerates every subclass, so it fails whenever test_1057 ran first in the
+same process.
+
+Underworld development team with AI support from Claude Code

src/underworld3/systems/solvers.py

@@ -63,6 +63,7 @@
 from underworld3 import VarType
 import underworld3.timing as timing
 from underworld3.utilities import memprobe
+from underworld3.utilities._petsc_tools import _fieldsplit_sub_ksps
 from underworld3.utilities._api_tools import (
     uw_object,
     SymbolicProperty,
@@ -2047,7 +2048,6 @@ def solve(
                 self._setup_pointwise_functions(verbose)
                 self._setup_discretisation(verbose)
                 self._setup_solver(verbose)
-                self._check_velocity_preconditioner()
 
             self._stress_history_advance(timestep, verbose=verbose, evalf=evalf)
 
@@ -2069,6 +2069,9 @@ def solve(
                 print(f"Stokes solver - store stress and shift history", flush=True)
 
             self._stress_history_post_solve(timestep, verbose=verbose, evalf=evalf)
+            # After the solve, as on the plain path: before it the split is not set
+            # up and has no velocity block to inspect (#827).
+            self._check_velocity_preconditioner()
 
         else:
             # Plain Stokes — no stress history
@@ -2715,20 +2718,16 @@ def _check_velocity_preconditioner(self):
         if getattr(self, "_pc_user_override", False):
             return                            # the user owns this block's pc_type
         # The rotated free-slip path builds its OWN fieldsplit KSP and leaves
-        # this one un-set-up, so asking it for sub-KSPs raises -- and PETSc
-        # prints a full error banner before the exception can be caught, which
-        # makes a healthy solve look broken. Skip it by the path rather than by
-        # catching. (Suppressing with pushErrorHandler was tried: it is global
-        # state and broke 23 unrelated tests.)
+        # this one un-set-up: there is nothing here to inspect.
         if getattr(self, "_rotated_freeslip_bcs", None):
             return
-        pc = self.snes.getKSP().getPC()
-        if pc.getType() != "fieldsplit":
-            return
-        try:
-            installed = pc.getFieldSplitSubKSP()[0].getPC().getType()
-        except Exception:
-            return                            # not set up: nothing to inspect
+        # A split that is not set up has no blocks -- after a rebuild, or a warm
+        # solve that converged before any linear solve -- and asking for them
+        # prints a PETSc banner even when caught (#827); the helper asks quietly.
+        blocks = _fieldsplit_sub_ksps(self.snes.getKSP().getPC())
+        if not blocks:
+            return                            # not a split, or not set up
+        installed = blocks[0].getPC().getType()
         if installed == "mg":
             return
 

src/underworld3/utilities/_petsc_tools.py

@@ -56,3 +56,34 @@ def requote(argument):
 
 
 import os as _os
+
+
+def _fieldsplit_sub_ksps(pc):
+    """The block solvers of a field-split preconditioner, asked for quietly.
+
+    A split has its blocks only once ``PCSetUp`` has run. Asking earlier is a
+    PETSc error, and PETSc prints its banner before the exception reaches Python,
+    so catching it alone cannot keep a healthy solve's output clean (#827). The
+    ``"return"`` error handler is pushed for this one call: the error is still
+    raised, it is just not printed, and the handler is popped on every path.
+
+    Parameters
+    ----------
+    pc : petsc4py.PETSc.PC
+        Any preconditioner.
+
+    Returns
+    -------
+    list of petsc4py.PETSc.KSP
+        The split's block solvers, or ``[]`` when ``pc`` is not a field split or
+        PETSc declines to give its blocks (the split is not set up).
+    """
+    if pc.getType() != "fieldsplit":
+        return []
+    PETSc.Sys.pushErrorHandler("return")
+    try:
+        return list(pc.getFieldSplitSubKSP())
+    except PETSc.Error:
+        return []                         # no blocks to give: not set up
+    finally:
+        PETSc.Sys.popErrorHandler()

src/underworld3/utilities/custom_mg.py

@@ -46,6 +46,7 @@
 from petsc4py import PETSc
 
 from underworld3.utilities import multigrid_options
+from underworld3.utilities._petsc_tools import _fieldsplit_sub_ksps
 
 __all__ = ["barycentric_prolongation", "rbf_prolongation", "inject_custom_mg",
            "CustomMGHierarchy", "set_custom_fmg", "sbr_refine", "sbr_refine_where",
@@ -2219,10 +2220,12 @@ def _pcmg_still_installed(solver, h):
         if pfx == "":
             pc = solver.snes.getKSP().getPC()
         elif pfx == "fieldsplit_velocity_":
-            outer = solver.snes.getKSP().getPC()
-            if outer.getType() != "fieldsplit":
+            # quietly: an un-set-up split has no blocks, and asking prints a
+            # PETSc banner even when caught (#827)
+            blocks = _fieldsplit_sub_ksps(solver.snes.getKSP().getPC())
+            if not blocks:
                 return False
-            pc = outer.getFieldSplitSubKSP()[0].getPC()
+            pc = blocks[0].getPC()
         else:
             return False
         return (pc.getType() == "mg"

tests/test_1077_quiet_unsetup_fieldsplit.py

@@ -0,0 +1,106 @@
+"""No PETSc error banner from asking an un-set-up field split for its blocks (#827).
+
+A field split has block solvers only once ``PCSetUp`` has run. Asking for them
+earlier raises, and PETSc prints a ~38-line "Object is in wrong state" banner to
+stderr before the exception reaches Python, so ``except`` cannot hide it and a
+healthy solve looks broken. Three paths reached it:
+
+- a viscoelastic solve checked the velocity preconditioner before solving;
+- the same check after a plain solve that converged without a linear solve, on
+  a solver rebuilt by a ``consistent_jacobian`` change, so the new split was
+  never set up;
+- the custom-multigrid "still installed?" test, run before each solve, on a
+  solver rebuilt by a ``consistent_jacobian`` change (the notch runs' path).
+
+The banner is C-level output on file descriptor 2, so ``capfd`` sees it.
+"""
+import pytest
+import sympy
+
+import underworld3 as uw
+from underworld3.utilities._petsc_tools import _fieldsplit_sub_ksps
+
+pytestmark = [pytest.mark.level_1, pytest.mark.tier_b]
+
+BANNER = "PETSC ERROR"
+
+
+def _stokes(cellSize=0.25, refinement=None):
+    mesh = uw.meshing.UnstructuredSimplexBox(
+        minCoords=(0.0, 0.0), maxCoords=(1.0, 1.0), cellSize=cellSize,
+        refinement=refinement)
+    v = uw.discretisation.MeshVariable("V", mesh, mesh.dim, degree=2)
+    p = uw.discretisation.MeshVariable("P", mesh, 1, degree=1)
+    stokes = uw.systems.Stokes(mesh, velocityField=v, pressureField=p)
+    stokes.add_dirichlet_bc((0.0, 0.0), "Bottom")
+    stokes.add_dirichlet_bc((1.0, 0.0), "Top")
+    return stokes
+
+
+def test_helper_is_quiet_on_an_unsetup_split(capfd):
+    """The helper: [] before set-up, with nothing printed; the blocks after."""
+    stokes = _stokes()
+    stokes.constitutive_model = uw.constitutive_models.ViscousFlowModel
+    stokes.constitutive_model.Parameters.shear_viscosity_0 = 1.0
+    stokes._setup_pointwise_functions()
+    stokes._setup_discretisation()
+    stokes._setup_solver()
+    pc = stokes.snes.getKSP().getPC()
+    assert pc.getType() == "fieldsplit"
+    assert _fieldsplit_sub_ksps(pc) == []
+    assert BANNER not in capfd.readouterr().err
+    stokes.solve()
+    assert len(_fieldsplit_sub_ksps(stokes.snes.getKSP().getPC())) == 2
+
+
+def test_viscoelastic_solve_prints_no_banner(capfd):
+    """The #827 reproducer."""
+    stokes = _stokes()
+    stokes.constitutive_model = uw.constitutive_models.ViscoElasticPlasticFlowModel(
+        stokes.Unknowns, order=1)
+    stokes.constitutive_model.Parameters.shear_viscosity_0 = 1.0
+    stokes.constitutive_model.Parameters.shear_modulus = 1.0
+    stokes.constitutive_model.Parameters.dt_elastic = sympy.Rational(1, 10)
+    stokes.solve(timestep=0.1)
+    assert BANNER not in capfd.readouterr().err
+
+
+def test_warm_solve_without_a_linear_solve_prints_no_banner(capfd):
+    """A tangent change rebuilds the solver; the warm solve that follows meets its
+    (loose) tolerance at once, so no linear solve sets the new split up, and the
+    post-solve velocity-preconditioner check asked it for its blocks."""
+    stokes = _stokes()
+    stokes.constitutive_model = uw.constitutive_models.ViscoPlasticFlowModel
+    stokes.constitutive_model.Parameters.shear_viscosity_0 = 1.0
+    stokes.constitutive_model.Parameters.yield_stress = 0.3
+    stokes.consistent_jacobian = False
+    stokes.solve()
+    capfd.readouterr()
+    stokes.petsc_options["snes_atol"] = 1.0e6
+    stokes.consistent_jacobian = True
+    stokes.solve(zero_init_guess=False)
+    assert stokes.snes.getIterationNumber() == 0
+    assert BANNER not in capfd.readouterr().err
+
+
+def test_custom_fmg_rewire_prints_no_banner(capfd):
+    """A custom multigrid on the velocity block (an independent, non-nested coarsest
+    mesh: the shape of the notch runs' gmsh-tail FMG), then a tangent change. The
+    change rebuilds the solver, and the next solve's "is the multigrid still
+    installed?" test asked the new, un-set-up split for its blocks."""
+    stokes = _stokes(refinement=1)
+    stokes.constitutive_model = uw.constitutive_models.ViscoPlasticFlowModel
+    stokes.constitutive_model.Parameters.shear_viscosity_0 = 1.0
+    stokes.constitutive_model.Parameters.yield_stress = 0.3
+    fine = stokes.mesh
+    tail = uw.meshing.UnstructuredSimplexBox(
+        minCoords=(0.0, 0.0), maxCoords=(1.0, 1.0), cellSize=0.6)
+    stokes.set_custom_fmg([tail] + fine._coarse_level_meshes()[:-1], field_id=0)
+    stokes.consistent_jacobian = False
+    stokes.solve()
+    capfd.readouterr()
+    stokes.consistent_jacobian = True
+    stokes.solve(zero_init_guess=False)
+    assert stokes.snes.getConvergedReason() > 0
+    assert BANNER not in capfd.readouterr().err
+    assert stokes.snes.getKSP().getPC().getFieldSplitSubKSP()[0].getPC().getType() == "mg"