Let a caller ask solve! to throw on a failed solve

[1-Bort-1]

TL;DR

solve! gains throw_on_fail, which turns a solve that missed the solver's tolerances or came back non-finite into a SolveFailure exception instead of a VSMSolution the caller has to inspect. The default is off, so nothing that solves today changes; SymbolicAWEModels.jl asks for the throw and is where this comes from (OpenSourceAWE/SymbolicAWEModels.jl#291).

Why the failure belongs here

A caller that cannot use a failed solve has to know that solver.lr.converged and sol.solver_status exist, and has to re-derive "did this come back finite" for itself — including over ForwardDiff.Dual partials, which is not obvious. SymbolicAWEModels.jl carried exactly that: its own VSMSolveFailure exception, its own Dual-aware finite_full, and three call sites that re-checked the solver's own flag and threw. Review on that PR said the error should exist and be thrown here, which is right: whether a solve is usable is this package's question, not a caller's.

So SolveFailure, the finite_full check and the throw are here now, and the caller says once that it wants them.

Why the default is off, and not on

Making solve! always throw was the first thing I tried, and two testsets in this repo say no. NONLIN solve! re-runs across calls solves at va = [10, 0, 5] — 26.6° — and solve! artificial viscosity: attached no-op, post-stall finite solves at 20° on a flat-plate LEI wing; both then assert on the result. Neither converges. The artificial-viscosity testset is explicit about what it wants there: a finite post-stall answer, not a converged one.

That is a real regime for this package, so the default keeps it: a post-stall solve still returns its solver_status == FAILURE solution and the caller decides. Flip the default if you would rather every caller be told — the two testsets above are what would have to move with it.

What I found while measuring that

On the wing test/solver/solver_test_wing.yaml builds, at va = [10, 0, 5], NONLIN misses the tolerances in all 1500 iterations while LOOP converges, and the two answers differ by 3.6% in peak circulation (max|gamma| 8.781 against 9.106). NONLIN solve! re-runs across calls has been asserting on that non-converged result. Nothing in this PR depends on it and I have not touched the test; filed as #283.

What the coverage check caught

codecov/patch was the one red check on the first push, and it was right: the lines it named are the ones only the calling package exercised. The throw fires on non-convergence before finite_full is ever reached, so both of its methods, showerror and the whole converged branch of throw_on_fail had no test here at all. They do now — including a Dual whose partial is not finite, which is the case the method exists for.

Verification

• Reproduced first: n/a — new feature
• test/solver/test_solver.jl red before (UndefVarError: SolveFailure not defined in Main), green after: 27 assertions, 9 of them new (juliaserver, Julia 1.12.7)
• Local full suite: PASS — test/runtests.jl, every testset green, one pre-existing @test_broken. The coverage commit that followed it only adds assertions to test/solver/test_solver.jl, which was re-run on its own: 27/27
• Docs build: DOCSBUILD OK
• Benchmark: n/a — the default path gains one throw_on_fail || return sol before the existing return
• Risk: the message a failure carries is the solver's, so a caller with several wings has to say which one it was itself.

Scope

+94 / −18 across 7 files: src/solver.jl is SolveFailure, finite_full and the kwarg; the rest is two testsets, two docs entries and the changelog. Not stacked on #273 — it shares no source file with this, only CHANGELOG.md.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[1-Bart-1]

Did you check if solver.lr.converged and sol.solver_status are actually updated and relevant and that it actually throws on a failing solve, is there a test?