Skip to content

feat(solvers): default-on solve reporting + bounded/resumable difficulty solve - #377

Merged
lmoresi merged 1 commit into
developmentfrom
feature/solver-report
Jul 27, 2026
Merged

feat(solvers): default-on solve reporting + bounded/resumable difficulty solve#377
lmoresi merged 1 commit into
developmentfrom
feature/solver-report

Conversation

@lmoresi

@lmoresi lmoresi commented Jul 22, 2026

Copy link
Copy Markdown
Member

What

Two general capabilities on SolverBaseClass, so every SNES-based solver gets them (Stokes, scalar, vector, and the rotated-free-slip KSP path):

1. Default-on difficulty reporting. Every solve() records a SolveReport (new pure-Python module systems/solve_report.py) with the converged reason, nonlinear/linear iteration counts, final and initial ||F||, residual reduction, contraction rho, function-evaluations, and the residual ladder. Read-only via solver.solve_report; a bounded solver.solve_history (deque, maxlen=32) keeps a short trail for continuation / time-stepping loops. Captured at the single solve chokepoint (_snes_solve_with_retries) so no solve() body needs editing; the rotated-free-slip path (which solves via ksp.solve, bypassing the chokepoint) gets an honest report from its own result rather than a stale one.

2. Bounded, resumable difficulty estimate. solver.estimate_difficulty(max_nl_its, warm=...) caps nonlinear iterations (predictable solver work, not wall-time), reports the effort spent, and continues losslessly from the partial iterate. A chunked start/stop/restart chain terminates at the same point an uninterrupted solve would: the chain anchors convergence to the original ||F0|| (absolute tol = tolerance*||F0||), which is looser than the restart-relative rtol*||F_restart|| and so fires first. Gated on an internal probe flag, so a plain solve() is byte-for-byte unchanged and cannot inherit a stale anchor.

Why

Groundwork for adaptive continuation / homotopy control: a solver should expose its difficulty and let a driver bound it to estimate difficulty cheaply, then resume without losing work. Reporting is default-on; the bounded/resumable knob is opt-in.

Safety

Behaviour of a normal solve is preserved -- the only additions on that path are a guarded setConvergenceHistory(reset=True) and a read-only report capture. estimate_difficulty restores tolerances after each call.

Tests

tests/test_1055_solve_report.py: default-on report on Stokes + scalar; bounded stop at the cap; lossless resume matching an uninterrupted reference; chunked chains terminate at the same ||F||.

Regression: level_1 and tier_a green -- 403 passed, 0 failed, 0 errors (1 pre-existing unrelated xfail).

Underworld development team with AI support from Claude Code

…lty solve

Two general capabilities on SolverBaseClass, so every SNES-based solver gets
them (Stokes, scalar, vector, and the rotated-free-slip KSP path):

1. Default-on difficulty reporting. Every solve() records a SolveReport
   (new pure-Python module systems/solve_report.py) exposing the converged
   reason, nonlinear/linear iteration counts, final and initial ||F||, the
   residual reduction, contraction rho, function-evaluations, and the residual
   ladder. Read-only via solver.solve_report; a bounded solver.solve_history
   (deque maxlen=32) keeps a short trail for continuation / time-stepping loops.
   Captured at the single solve chokepoint (_snes_solve_with_retries) so no
   solve() body needs editing; the rotated-free-slip path (which solves via
   ksp.solve, bypassing the chokepoint) gets an honest report from its own
   result rather than a stale one.

2. Bounded, resumable difficulty estimate. solver.estimate_difficulty(
   max_nl_its, warm=...) caps NONLINEAR iterations (predictable solver work,
   not wall-time), reports the effort spent, and continues losslessly from the
   partial iterate. A chunked start/stop/restart chain terminates at the SAME
   point an uninterrupted solve would: the chain anchors convergence to the
   original ||F0|| (absolute tol = tolerance*||F0||), which is looser than the
   restart-relative rtol*||F_restart|| and so fires first. Gated on an internal
   probe flag, so a plain solve() is byte-for-byte unchanged and cannot inherit
   a stale anchor.

Reporting is default-on; the bounded/resumable knob is opt-in. Behaviour of a
normal solve is preserved (only additions on that path are a guarded
setConvergenceHistory and a read-only report capture).

Tests: tests/test_1055_solve_report.py (default-on report on Stokes + scalar;
bounded stop at the cap; lossless resume matching an uninterrupted reference;
chunked chains terminate at the same ||F||). Regression: level_1/tier_a green
(403 passed, 0 failed, 0 errors).

Underworld development team with AI support from Claude Code
Copilot AI review requested due to automatic review settings July 22, 2026 00:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@lmoresi
lmoresi merged commit 04898b1 into development Jul 27, 2026
2 of 3 checks passed
@lmoresi
lmoresi deleted the feature/solver-report branch July 27, 2026 04:52
@lmoresi

lmoresi commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

Post-merge adversarial review (04898b1)

This PR merged without its public adversarial review; this is that review, run post-merge against the squash commit on development. An independent skeptic session probed every claim in the merge message with live serial and np2/np4 runs (probe scripts referenced by name; environment verified against site-packages before every probe). Findings will be addressed in an immediate follow-up bugfix PR.

Verdict: 12 findings — 3 MAJOR, 3 MINOR, 6 NOTE. No CRITICAL. The core machinery (plain-solve isolation, chunked-chain equivalence, per-instance state, parallel consistency) survived every attack. The rotated-free-slip reporting — explicitly claimed in the merge message — is broken on the nonlinear path, and the resume-anchor has an unmanaged lifecycle.


Findings (ranked)

1. MAJOR — Nonlinear rotated free-slip solve leaves solve_report = None; the claimed "honest report from its own result" does not exist on that path — PROBED

_capture_rotated_report (petsc_generic_snes_solvers.pyx:1022-1044) reads the wrong keys/types from the nonlinear result dict returned by solve_rotated_freeslip_nonlinear:

  • int(info.get("ksp_its", 0)) — but the nonlinear dict's "ksp_its" is a list (one count per Newton increment) → TypeError → swallowed by the blanket except Exceptionself._solve_report = None.
  • info.get("newton_its", 1) — the dict's actual key is "nonlinear_iterations", so even with the type fixed, nl_its would always be 1.
  • converged = reason > 0 uses the last KSP reason; the dict's outer-Newton "converged" flag is ignored — a chain whose final linear solve converged but whose Newton loop stalled would be reported converged.

Probe (annulus, add_rotated_freeslip_bc both walls, strain-rate-dependent viscosity): a successful 7-iteration nonlinear rotated solve leaves no report and an empty history. The blanket "best-effort, never raises" swallow is precisely what let this ship silently.

2. MAJOR — estimate_difficulty on a rotated free-slip solver silently ignores the cap and returns None — PROBED

The rotated branch in SNES_Stokes_SaddlePt.solve returns before _snes_solve_with_retries, so _difficulty_max_it is never applied — the "bounded" probe runs the full unbounded Newton loop (probe: cap=1 requested, 7 Newton iterations actually run, report None). Combined with finding 1, any caller doing rep.bounded/rep.nl_its gets an AttributeError on NoneType. Silently running 7× the requested work inverts the feature's purpose ("predictable solver work"). Should honour the cap or raise.

3. MAJOR — Resume-anchor lifecycle: poisoned by a warm first call, never invalidated by convergence, physics changes, or solver rebuild — PROBED

estimate_difficulty sets the chain anchor on the first call where _resume_abs_target is None, regardless of field state, and nothing except an explicit warm=False ever clears it:

  • Poisoning: after a converged plain solve, estimate_difficulty(max_nl_its=10, warm=True) — the natural "how hard was that?" call the docstring invites — converges in 0 iterations and sets anchor = tolerance*‖F_restart‖ = 9.45e-14 (vs the meaningful 9.99e-8). Effectively no anchor: the chain's same-termination guarantee silently degrades to restart-relative rtol semantics. The next probe ground 3 extra Newton+KSP iterations on an already-converged problem and exited with a misleading CONVERGED_ITS.
  • No invalidation across physics changes: after changing bodyforce ×0.01 — which even rebuilt the SNES object — the old anchor from the previous problem's ‖F0‖ was still armed in the new solve.

Fix shape: clear _resume_abs_target when a probe converges (reason > 0); refuse to establish a chain from a warm call whose initial residual already satisfies the current tolerance.

4. MINOR — Linear rotated report is mostly hollow, and KSP reason codes are rendered with SNES reason names — PROBED (hollow) / verified against petsc4py constants (mislabel)

Probe: SolveReport(CONVERGED_FNORM_RELATIVE, nl=1, ksp=1, |F|=nan, …), fnorm0=None, empty ladder — "rnorm" is never in the linear result dict, so fnorm is always NaN on this path; nl_its is hardcoded 1. More seriously, reason is a KSP code passed through a SNES reason table: KSP_DIVERGED_MAX_IT (-3) prints as DIVERGED_LINEAR_SOLVE, KSP_DIVERGED_DTOL (-4) as DIVERGED_FNORM_NAN, KSP_DIVERGED_NANORINF (-9) as DIVERGED_DTOL, KSP_CONVERGED_ATOL (3) as CONVERGED_SNORM_RELATIVE. A diagnostic tool that mislabels failure modes is worse than none.

5. MINOR — A solve that raises leaves the previous, converged report in solve_report — PROBED

_capture_solve_report runs after the try/finally; an exception propagating out of snes.solve skips it. Probe: after an injected failure, solve_report still shows the earlier solve's converged=True. Any recovery logic that catches the exception and consults the report is misled. Capture in the finally (or null the report at solve entry).

6. MINOR — Style Charter §4: uncommented exception swallows; the blanket swallow in _capture_rotated_report actively masked finding 1 — READING

Three try/except Exception blocks in _capture_solve_report state no sanctioned failure mode; _capture_rotated_report's whole-body swallow converted a deterministic type bug into silent data loss. Rest of the new code is Charter-clean.

7. NOTE — Under consistent_jacobian="continuation", the cap applies per snes.solve: up to 2× max_nl_its per probe (Picard stage + Newton stage each inherit the cap) — READING

8. NOTE — divergence_retries forwarded through estimate_difficulty multiplies the cap — a capped probe ends DIVERGED_MAX_IT, exactly what the retry loop retries on (up to 3× the cap). Also estimate_difficulty(2, zero_init_guess=True) raises a bare duplicate-kwarg TypeError. — READING

9. NOTE — Anchor uses self.tolerance, not the live SNES rtol: a user setting snes_rtol via petsc_options quietly breaks the same-termination equivalence. — READING

10. NOTE — solve_history returns the live internal deque (.clear() succeeds); return a copy or document. — PROBED

11. NOTE — Test gaps map exactly onto the escaped defects: zero rotated-path coverage in test_1055 despite the merge-message claim (findings 1, 2, 4 live there); nothing probes a warm-start chain from a converged state (finding 3); one assertion is conditional on if r.fnorm0:. Credit: the lossless-resume test is genuinely sensitive — with the anchor absent the resumed solve would overshoot ~100×, which the check would catch. — READING

12. NOTE — setConvergenceHistory(reset=True) on every solve silently replaces any user-armed history buffer (bounded cost, no leak). — READING


Attacks that failed (confirmations, all PROBED)

  • Chunked chain == uninterrupted (the headline claim): iterate ladders concatenate exactly; final fields bit-identical (rel err 0.0), fnorm ratio 1.000; anchor atol confirmed live on the SNES during the resumed solve and firing at the correct iterate.
  • Plain-solve isolation: after a probe chain, snes.getTolerances() exactly restored; a plain solve on the probed solver reproduces a never-probed reference to rel err 1.6e-15 with identical nl_its/fnorm.
  • Class-level-deque bug: histories are per-instance; solving one solver leaves another's report None.
  • Ladder accumulation: per-solve reset works; 3 consecutive solves each report a clean 2-point ladder; history grows by exactly one per solve.
  • ‖F0‖ = 0 edge: fnorm0=0.0, reduction=None, anchor correctly not set, no NaN blowup.
  • Parallel: np2 and np4 — plain, bounded, and resume reports plus the anchor bitwise identical across ranks (allgather repr-compare incl. full ladder); rotated linear annulus included; no hangs.
  • Memory: bounded by construction (deque maxlen 32; per-solve buffer reused).
  • UWQuantity leakage: every report field passes through int()/float(); no path admits a Quantity.

Follow-up (bugfix PR imminent)

  1. Fix _capture_rotated_report key/type contract against both rotated dicts; add a KSP-namespace reason table; remove the blanket swallow (1, 4, 6).
  2. estimate_difficulty on the rotated path: honour the cap or raise NotImplementedError (2).
  3. Anchor lifecycle: clear on converged probe; don't establish a chain from an already-converged warm state (3).
  4. Add the missing tests: rotated-path reports (linear + nonlinear), warm-first-call chain (11).

Adversarial review per project policy — Underworld development team with AI support from Claude Code

lmoresi added a commit that referenced this pull request Jul 27, 2026
… anchor lifecycle (PR #377 review) (#437)

Fixes the three MAJOR and three MINOR findings from the post-merge adversarial
review of PR #377:

1. _capture_rotated_report now honours both rotated result dicts: the nonlinear
   loop's ksp_its LIST (summed), nonlinear_iterations, and the outer converged
   verdict. Previously a TypeError on the list was swallowed by a blanket
   except, leaving solve_report = None after every nonlinear rotated solve.
   The blanket swallow is gone: a malformed dict now raises (the dicts and the
   reader are a contract; the swallow is what masked the breakage).

2. KSP converged-reason codes get their own table (KSP_ prefix) — they were
   rendered with SNES names that share integers but mean different outcomes
   (e.g. -3: KSP DIVERGED_MAX_IT vs SNES DIVERGED_LINEAR_SOLVE). A test pins
   the table against petsc4py's enum.

3. Both rotated paths report a real residual: the linear solve computes the
   true rotated residual (preonly/LU never computes a norm; iterative norms
   can be preconditioned), the nonlinear loop returns its final/initial |F|.

4. estimate_difficulty raises NotImplementedError with rotated free-slip BCs
   (that path solves outside self.snes, so the cap and anchor cannot reach
   it — it previously ran unbounded and returned None), and rejects
   zero_init_guess / divergence_retries kwargs that would break the probe
   contract.

5. Anchor lifecycle: a CONVERGED probe clears the resume anchor (a warm first
   call on a converged state previously armed an unreachable target at
   tolerance*|F_converged| and ground extra iterations with a misleading
   exit), and _reset clears it (a torn-down solver is a different problem).

6. Report capture moved into the finally of _snes_solve_with_retries: a solve
   that raises now leaves an honest report of the failed attempt instead of
   the previous solve's converged one.

Tests: 5 new cases in test_1055 (rotated linear + nonlinear reports, rotated /
kwarg rejection, converged-warm anchor hygiene, KSP table pinned to petsc4py).

Underworld development team with AI support from Claude Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants