Count sub-solves that stop at their iteration cap (#625) - #631
Merged
Conversation
… pairing PETSc's KSPCheckSolve deliberately does not treat DIVERGED_MAX_IT on a sub-KSP as a failure, so a block can run out of iterations on every application and the outer solve still reports CONVERGED. That silence is what #625 is made of. It is expensive because S = -B A^-1 B^T is applied THROUGH the velocity solve. Truncate those and the pressure Krylov is chasing an operator that moves between applications, so it cannot converge either and caps in turn. Measured on SolCx at h=1/30, changing only fieldsplit_velocity_ksp_max_it from 200 to 5000: 976 s to 25.6 s, outer 44 to 2, pressure 200/application to 30, and an identical velocity error. Thirty-eight times, from a number that is not in the pressure block. solve_report.sub[...].capped now counts the applications that ended that way, read from each application's converged reason via a post-solve hook rather than by comparing an iteration count against the cap -- a solve that converges on its last permitted iteration is converged, and counting would misreport it. The skill records the pairing this exposed: with FMG, penalty=10 improves every axis at once (21% faster, Schur count 3.3x smaller, total velocity work halved), because FMG absorbs grad-div augmentation where GAMG is driven into its cap by it. With GAMG the same penalty makes the solve slower, so the two must not be recommended independently. The accuracy cost is a consistent perturbation -- same convergence rate, gap shrinking 1.102 -> 1.087 -> 1.066 under refinement -- which is what makes the pairing safe to recommend at all. No numerical behaviour changes here: the counter is observation, and the defaults are untouched. Underworld development team with AI support from Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
PETSc's
KSPCheckSolvedeliberately does not treatDIVERGED_MAX_ITon asub-KSP as a failure — truncating an inner solve is normal. So a block can run
out of iterations on every application and the outer solve still reports
CONVERGED. Nothing in the output says otherwise.
That silence is what #625 is made of, and it is expensive because
S = -B A^-1 B^Tis applied through the velocity solve. Truncate those andthe pressure Krylov is chasing an operator that moves between applications, so
it cannot converge either and caps in turn. Measured on SolCx at h=1/30,
changing only
fieldsplit_velocity_ksp_max_it:38x, from a number that is not in the pressure block, with an identical
answer. Every pathology measured in that campaign turned out to be a cap: the
pressure block, the velocity block beneath it, and the velocity block again
under augmentation. All three would have been visible on sight.
The change
solve_report.sub[...].cappedcounts the applications that ended at the cap.The count comes from each application's converged reason, via a post-solve
hook — not from comparing an iteration count against
max_it, because a solvethat converges on its last permitted iteration is converged and counting would
misreport it.
str(report)gains— N AT THE ITERATION CAPwhen non-zero.No numerical behaviour changes. The counter is observation, and no default
is touched.
Also: the FMG/penalty pairing, recorded in the
nonlinear-solverskillThe same campaign established a pairing worth writing down. Matched on one mesh
(2592 cells), same discrete solve, only the velocity preconditioner differing:
With FMG,
penalty = 10improves every axis at once. With GAMG the same valuemakes the solve slower, because grad-div augmentation is exactly what drives
GAMG into its cap. The two cannot be recommended independently.
The accuracy cost is a consistent perturbation rather than a changed answer —
same convergence rate, gap shrinking 1.102 → 1.087 → 1.066 across three
refinements — which is what makes the pairing safe to recommend. UW3's penalty
is grad-div, not a true augmented Lagrangian (
div(P2)is not insideP0), soit does perturb the discrete solution; it just converges away.
The default is deliberately NOT changed here. Flipping
penaltyon for FMGwould alter answers by ~7% at working resolutions, and the evidence so far is
one 2-D linear benchmark. See the issue for what still needs measuring.
Testing
tests/test_0205_subsolve_cap_detection.py— 3 tests: a hard cap is counted onevery application, a cap never approached reports zero (the negative control,
without which a counter stuck at
applicationswould pass), and the countdistinguishes some-from-all rather than collapsing to a boolean.
Existing instrumentation suites unaffected:
test_0203,test_0204,test_1017— 18 passed.Underworld development team with AI support from Claude Code