fix(solve-report): SNES converged-reason table is off by one against PETSc#444
Open
lmoresi wants to merge 1 commit into
Open
fix(solve-report): SNES converged-reason table is off by one against PETSc#444lmoresi wants to merge 1 commit into
lmoresi wants to merge 1 commit into
Conversation
… PETSc Every positive SNES reason was shifted by one slot, and the table claimed a code 1 that PETSc does not define: reason 2 PETSc CONVERGED_FNORM_ABS reported as CONVERGED_FNORM_RELATIVE reason 3 PETSc CONVERGED_FNORM_RELATIVE reported as CONVERGED_SNORM_RELATIVE reason 4 PETSc CONVERGED_SNORM_RELATIVE reported as CONVERGED_ITS reason 5 PETSc CONVERGED_ITS reported as UNKNOWN_5 The direction matters. A solve that stopped on the STEP norm -- the weakest criterion, and what a stalled viscoplastic solve reports -- was labelled CONVERGED_ITS, while a genuine residual convergence was labelled CONVERGED_SNORM_RELATIVE. Reading a difficulty report is how a continuation driver decides whether a parameter station is reachable, so a mislabelled convergence is the kind of error that quietly puts false rescues on a regime map. Found while checking why a hard plastic station was reporting CONVERGED_ITS. Also adds DIVERGED_OBJECTIVE_DOMAIN (-13) and DIVERGED_OBJECTIVE_NANORINF (-14), which were unmapped and surfaced as UNKNOWN_n, and corrects -4 to its PETSc name DIVERGED_FUNCTION_NANORINF. The table is hand-written on purpose -- the module must import without the Cython extension -- so it can drift, and the existing test pinned it to ITSELF (reason_string(2) == 'CONVERGED_FNORM_RELATIVE') rather than to the enum, which is how this survived. The KSP table next to it WAS checked against petsc4py and was correct. test_snes_reason_table_matches_petsc now checks both directions: every label matches the enum, and every reason PETSc can return is mapped. 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.
The bug
solve_report's SNES converged-reason table is shifted by one against PETSc, and claims acode
1that PETSc does not define:CONVERGED_FNORM_ABSCONVERGED_FNORM_RELATIVECONVERGED_FNORM_RELATIVECONVERGED_SNORM_RELATIVECONVERGED_SNORM_RELATIVECONVERGED_ITSCONVERGED_ITSUNKNOWN_5Why the direction matters
A solve that stopped on the step norm — the weakest criterion, and what a stalled
viscoplastic solve reports — was labelled
CONVERGED_ITS. A genuine residual convergencewas labelled
CONVERGED_SNORM_RELATIVE.Reading a difficulty report is how a continuation driver decides whether a parameter
station is reachable, so a mislabelled convergence is exactly the kind of error that puts
false rescues on a regime map without anyone noticing.
Found while checking why a hard Drucker–Prager station was reporting
CONVERGED_ITSon asolve that had no iteration cap in play. It had actually converged on the step norm — a
different and much weaker claim.
Also fixed
DIVERGED_OBJECTIVE_DOMAIN(-13) andDIVERGED_OBJECTIVE_NANORINF(-14) were unmappedand surfaced as
UNKNOWN_n; -4 isDIVERGED_FUNCTION_NANORINFin PETSc, notDIVERGED_FNORM_NAN.How it survived
The table is hand-written on purpose — the module must import without the Cython
extension present — so it can drift. The existing test pinned it to itself:
The KSP table sitting next to it was checked against
petsc4pyand is correct. Only theSNES one was never pinned.
test_snes_reason_table_matches_petscnow checks both directions: every label matches theenum, and every reason PETSc can return is mapped — so a PETSc renumbering or a future
hand-edit fails the test instead of silently mislabelling solves.
Verified against
petsc4py3.25; the reason-table tests pass.Underworld development team with AI support from Claude Code