next_step! should gain a kwarg that makes it warn on failing vsm solve

[1-Bort-1]

TL;DR

next_step!(sam; vsm_warn_on_fail=true) turns a failed VSM solve into a warning and flies the wing on with the circulation, the angles of attack and the frozen forces of its last converged solve, instead of ending the run. The failure itself is now VortexStepMethod.SolveFailure, raised by VSM's own solve! — this round moved it there, per the review comment on common.jl.

What a failed solve used to cost

safe_vsm_solve! zeroed the circulation on a failure and every mode's refresh threw an AssertionError, so one bad step ended the simulation and the next solve started from zero rather than from the last good circulation. Both halves are fixed. The last converged circulation and both angle-of-attack distributions are put back before the failure propagates — solve! has already overwritten them with the diverged ones by the time anyone can look — and vsm_warn_on_fail downgrades the failure to a warning. vsm_interval is untouched, so the next scheduled update solves again; the assertions on an already corrupted frozen state stay fatal, because a wing whose point forces are NaN cannot be flown on.

AeroLinearized is the one place where "nothing frozen is written before the solve can fail" is not true, and docs/src/vsm_coupling.md says so: a solve that fails during the Jacobian pass keeps the baseline coefficients of the operating point it just refreshed, next to the Jacobian columns the pass had not reached.

Where the error lives now

Review said the error should exist and be thrown in VortexStepMethod, not here. It does: OpenSourceAWE/VortexStepMethod.jl#282 adds SolveFailure and solve!(...; throw_on_fail=true), and takes this package's Dual-aware finite_full with it — whether a solve came back usable is VSM's question, not a caller's. This branch deletes VSMSolveFailure and finite_full, the three if !safe_vsm_solve!(...) throw(...) blocks collapse to plain calls, safe_vsm_solve! catches, restores and rethrows, and warn_or_rethrow names the mode and the wing that VSM's own message cannot. src/aero_modes/common.jl gets shorter by doing it.

The throw is opt-in over there, not the default: two VSM testsets solve deliberately non-converging post-stall cases and assert on the result, one of them explicitly wanting a finite answer rather than a converged one. The reasoning is on that PR, and a NONLIN non-convergence it turned up is OpenSourceAWE/VortexStepMethod.jl#283.

This PR cannot merge before that one is released. SymbolicAWEModels pins VortexStepMethod = "4.3" and its tracked manifest 4.3.1, while VSM main is already 5.0.0, so the compat entry and Manifest-v*.default both have to move onto the release that carries SolveFailure — a dependency bump of its own (AGENTS §7), not this diff. Project.toml is untouched here; it was relaxed only locally, to resolve against the dev'd worktree, and restored before the commit. Until that release exists, CI on this PR fails at throw_on_fail.

The one file the local suite is run without

test/test_helpers.jl asserts that both tracked Manifest-v1.1x.toml.default are no older than Project.toml, and it compares mtimes. Project.toml here is byte-identical to main — its last commit is cb482f1e, on the base — but Pkg rewrote it when the VortexStepMethod worktree was deved for this round, so it now sits 18 hours ahead of the manifests: mtime(Manifest-v1.12.toml.default) = 1.7888818752933135e9 against project_mtime = 1.788948339274766e9. A git archive of the three files into an empty directory satisfies both assertions, which is what a CI checkout produces — and the CI cells that ran green on this branch in round 4 included that file. That is a false positive on any worktree with a deved dependency, which the cross-repo workflow requires; filed as #305 rather than fixed here.

Verification

• Reproduced first: n/a — new kwarg
• VortexStepMethod side: test/solver/test_solver.jl red before (UndefVarError: SolveFailure not defined in Main), green after — 27 assertions; full suite test/runtests.jl PASS; docs build clean
• test/test_aero_modes.jl 809/809 (2m00.4s) against the dev'd VortexStepMethod — every model rebuilt from scratch, the caches were deleted first
• Local full suite: PASS — 3331 assertions in 26m04.2s, Julia 1.12.7, every file except test_helpers for the reason above
• Docs build and agent ci-local: not run — both install the dependency from the tracked manifest, which pins VortexStepMethod 4.3.1. The last docs build on this branch failed only on the base's private_functions.html size threshold (reported on AeroPressure: polars regenerated from the deformed shape, not a flap angle #285, now Split the private API doc pages; three are over Documenter's warn threshold #301), and this change takes two entries off that page. bin/reuse_lint still has no reuse on the box
• GitHub CI: red until VortexStepMethod is released — see above
• Benchmark: refresh_aero! on the 10-panel bench model is 391232 B and 0.203 ms median, of which the three distributions this change snapshots are 416 B and 0.275 µs — 0.11% of the allocations, 0.14% of the time. That is the whole cost on the success path; the rest of the diff is a try that does not throw. Box: steady — 6 of 20 cores busy, 1.4–3.4 GHz, 23.6 GB free, no swap pressure
• Risk: the warning text changed, so anything grepping logs for "VSM solve failed" now wants "Reusing the last converged aero state".

Scope

+147 / −61 across 8 files, of which src/aero_modes/common.jl is most of both sides: the exception and finite_full leaving, and safe_vsm_solve! becoming a try/restore/rethrow. Its three-branch gamma_init dispatch folds into one call while I am in there — solve!'s positional default is solver.sol.gamma_distribution. Second PR: OpenSourceAWE/VortexStepMethod.jl#282, which merges and releases first.

Opened elsewhere

• Pull request: Let a caller ask solve! to throw on a failed solve VortexStepMethod.jl#282
• Issue: NONLIN misses the tolerances at 26.6° where LOOP converges VortexStepMethod.jl#283

Closes #290 · task SymbolicAWEModels.jl-290

[1-Bort-1]

Independent review (advisory)

Verdict: APPROVE WITH COMMENTS · 2 inline, 0 off the diff

Good

• safe_vsm_solve! copies gamma before the solve and copyto!-restores it on failure (lines 1550-1568), because solve! clobbers gamma_distribution -- matches the PR's core claim exactly.
• VSMSolveFailure replaces AssertionError only at the VSM-convergence throw sites; the non-finite point-force/traction assertions stay AssertionError (direct.jl:120, pressure.jl:465), honoring the Decisions.
• warn_or_rethrow is a single shared codepath for both the rigid and particle loops and rethrows non-VSM failures, so unrelated exceptions still crash (common.jl:953-957).
• The test asserts solver.lr.gamma_new != gamma (line 111), proving the failed solve actually moved the iterate rather than masking a silent no-op.
• Reuses system_name="wing_test_REFINE" (line 84) so the model cache is shared with test_wing.jl, per rubric §8.

Not good

• docs/src/vsm_coupling.md:258 — "throws ... before writing any frozen state" is false for AeroLinearized: rigid_aero_baseline! writes wing.aero_x and ForwardDiff leaves partial aero_jac before the solve can fail, so the doc overstates the guarantee the card's Risk section admits.
• test/test_vsm_solve_failure.jl:9 — The "forced through the solver rather than through a pose" rationale is design history that belongs in the PR description, not a comment block in code.
• The try/catch+warn_or_rethrow block is duplicated verbatim for the rigid (918-923) and particle (935-940) loops; it could be one hoisted helper.
• The test-file header (lines 4-11) restates rationale that already lives in the PR Decisions (force non-convergence via rtol=0), against §3/§5.
• The unconditional copy per solve (1550-1551) allocates even on converging solves; disclosed in the card but not obviously avoidable since it must precede solve!.

_{opencode, rubric CLEAN_CODE.md. A different lab from the implementer
on purpose: a reviewer sharing its blind spots would not flag its mistakes.}

[1-Bort-1]

MINOR: The "forced through the solver rather than through a pose" rationale is design history that belongs in the PR description, not a comment block in code.

[1-Bart-1]

@1-Bort-1 fix this

[1-Bort-1]

Fixed. The claim now names what is actually restored — the circulation and the two angle-of-attack distributions — and docs/src/vsm_coupling.md states the AeroLinearized exception once: a solve that fails during the Jacobian pass keeps the baseline coefficients of the operating point it just refreshed, next to the Jacobian columns the pass had not reached. refresh_aero!, warn_or_rethrow and the next_step! docstring lost the absolute wording. What is true for every mode, and still stated, is that the forces stay those of the last converged solve: apply_direct_forces! runs after the Jacobian, so a failure there never rewrites them.

[1-Bart-1]

@1-Bort-1 can't this just be added to an existing test? This is a lot of lines of code for just one kwarg to be tested.

[1-Bort-1]

Folded in, file deleted. The contract is now driver (C) failed VSM solve inside the per-case loop of test_aero_modes.jl, gated on case.reference == :vsm: ~30 lines, no model of its own, and it runs on all five VSM cases (direct particle, direct rigid, continuous, pressure, linearized rigid) rather than the one the standalone file built — 809 assertions where the file had 779. Net 105 lines lighter. It also dropped the using Logging that had made the full suite red under Pkg.test; the counting now goes through Test.collect_test_logs.

[1-Bart-1]

@1-Bort-1 rebase this onto feat/live-polars and stack the PR

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 30 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/aero_modes/common.jl	0.00%	26 Missing ⚠️
src/symbolic_awe_model.jl	0.00%	2 Missing ⚠️
src/aero_modes/direct.jl	0.00%	1 Missing ⚠️
src/simulate.jl	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[1-Bart-1]

merge main into here, fix conflicts @1-Bort-1

[1-Bart-1]

This error should exist and be thrown in vsm, not in symbolicawemodels

[1-Bort-1]

Moved: OpenSourceAWE/VortexStepMethod.jl@544badd defines SolveFailure and throws it from solve!(...; throw_on_fail=true), and takes finite_full with it; this branch only catches now (50734e52). The throw is opt-in rather than the default because NONLIN solve! re-runs across calls and solve! artificial viscosity: attached no-op, post-stall finite both solve non-converging post-stall cases and assert on the result — say the word and I will flip it and move those two.

[1-Bort-1]

CI: Julia 1.12 - ubuntu-latest - x64 - monolith - pull_request failed on 50734e525.

https://github.com/OpenSourceAWE/SymbolicAWEModels.jl/actions/runs/34338389787/job/102423090619

[1-Bort-1]

This check fails on the dependency chain, not the code. CI installs the registered VortexStepMethod 4.3.1 (this branch's [compat] is still "4.3"), which has neither SolveFailure nor throw_on_fail — both exist only in the un-merged OpenSourceAWE/VortexStepMethod.jl#282. So SymbolicAWEModels cannot precompile: UndefVarError: SolveFailure not defined in VortexStepMethod. #291 is the dependent; it stays red until #282 merges, VSM releases, and the compat/manifest bump lands (a dependency bump that is not this PR's). I've set #291 back to draft until that release exists, and retracted my earlier comment blaming test_helpers' mtime staleness — the actual failure here is the precompile one.

[1-Bort-1]

Local full suite: FAIL (14 min, Julia 1.12.7, one cell of the matrix)

  Deserialization Tests           |   10           10     13.3s
  flap + continuous pressure aero |   18           18   1m25.4s
  Flap δ + beam-anchored point    |   28           28     24.4s
  Flow curvature                  |   76           76     57.8s
  Getter allocations              |    6            6     23.6s
  Getter allocations with a wing  |    4            4     15.9s
  Heading - Circular Path         |   72           72      0.2s
  Heading - Horizontal Circle     |   48           48      0.1s
  Heading - Special Cases         |   22           22      0.3s
  Testing helper functions...     |    7     1      8      1.5s
RNG of the outermost testset: Random.Xoshiro(0xb7582a3742bff233, 0x7a14a683fdeec112, 0xc8a6871a6c4d9902, 0x8fcb678cd651fec2, 0xff669621364e29c1)
ERROR: LoadError: Some tests did not pass: 1276 passed, 1 failed, 0 errored, 0 broken.
in expression starting at /home/agent/worktrees/SymbolicAWEModels.jl-290/test/runtests.jl:75
ERROR: Package SymbolicAWEModels errored during testing
Stacktrace:
  [1] pkgerror(msg::String)
    @ Pkg.Types ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/Types.jl:68
  [2] test(ctx::Pkg.Types.Context, pkgs::Vector{PackageSpec}; coverage::Bool, julia_args::Cmd, test_args::Cmd, test_fn::Nothing, force_latest_compatible_version::Bool, allow_earlier_backwards_compatible_versions::Bool, allow_reresolve::Bool)
    @ Pkg.Operations ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/Operations.jl:2642
  [3] test
    @ ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/Operations.jl:2487 [inlined]
  [4] test(ctx::Pkg.Types.Context, pkgs::Vector{PackageSpec}; coverage::Bool, test_fn::Nothing, julia_args::Cmd, test_args::Cmd, force_latest_compatible_version::Bool, allow_earlier_backwards_compatible_versions::Bool, allow_reresolve::Bool, kwargs::@Kwargs{io::IOContext{IO}})
    @ Pkg.API ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/API.jl:552
  [5] test(pkgs::Vector{PackageSpec}; io::IOContext{IO}, kwargs::@Kwargs{})
    @ Pkg.API ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/API.jl:169
  [6] test(pkgs::Vector{PackageSpec})
    @ Pkg.API ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/API.jl:158
  [7] test(; name::Nothing, uuid::Nothing, version::Nothing, url::Nothing, rev::Nothing, path::Nothing, mode::PackageMode, subdir::Nothing, kwargs::@Kwargs{})
    @ Pkg.API ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/API.jl:186
  [8] test()
    @ Pkg.API ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/API.jl:175
  [9] top-level scope
    @ none:1
 [10] eval(m::Module, e::Any)
    @ Core ./boot.jl:489
 [11] exec_options(opts::Base.JLOptions)
    @ Base ./client.jl:283
 [12] _start()
    @ Base ./client.jl:550