Skip to content

NONLIN misses the tolerances at 26.6° where LOOP converges - #285

Merged
1-Bart-1 merged 1 commit into
mainfrom
agent/283-nonlin-misses-the-tolerances-at-26-6-whe
Sep 10, 2026
Merged

1-Bart-1 merged 1 commit into
mainfrom
agent/283-nonlin-misses-the-tolerances-at-26-6-whe

Conversation

@1-Bort-1

@1-Bort-1 1-Bort-1 commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

TL;DR

NONLIN's Newton iteration took the full step every time, and past stall that overshoots into a limit cycle instead of converging — so at 26.6° it burned all 1500 iterations and returned an answer that is not a fixed point at all. It now backtracks to the largest fraction of the step that brings the residual down, and converges there in five iterations onto the distribution LOOP finds.

Which solver was wrong

The issue's read — "both solvers are solving the same fixed point, so one of them is wrong about having found it" — is right, and it is NONLIN. Evaluating the fixed-point map once from each solver's own answer (relaxation_factor=1, max_iterations=1, from that answer) gives the residual max|F(gamma) - gamma| each one actually stopped at:

aoa NONLIN, before LOOP
0.0° 1.4e-15 3.0e-4
5.7° 1.5e-12 3.0e-4
11.3° 1.3e-12 3.0e-4
16.7° 2.7e-16 3.1e-4
26.6° 0.589 3.1e-4

(relative to max|gamma|.) LOOP's answer at 26.6° is a fixed point to 3e-4; NONLIN's is off by 59% of its own peak circulation. The 3.6% gap in max|gamma| the issue measured understates how far away it was.

Why the Newton step cycles

Replaying the iteration outside the solver, from the same elliptical start, it is not slow convergence — it is a period-3 limit cycle: max|r| runs 5.17 → 16.38 → 5.79 → 5.17 forever, with cond(J) between 1.2 and 2.8 the whole way. A well-conditioned Jacobian ruling out ill-conditioning leaves overshoot: full steps of 3 to 6 while the solution sits near gamma = 9.1. The fix is the standard one — accept a step only if it reduces max|residual|, halving up to six times otherwise.

Two details keep it free where the old step already worked. The accepted trial's residual becomes the next iteration's, replacing the residual evaluation the loop used to do at the end of its body, so an undamped iteration makes exactly as many calls into update_gamma_candidate! as before. And convergence still tests the full Newton step, not the damped one, so a backtracked step can never fake convergence by being small.

What it does to the rest of the sweep

Every point that converged before converges to the same answer, bit for bit (max|gamma| 2.598 / 3.9344 / 5.2655 / 6.5404 unchanged at 0°/5.7°/11.3°/16.7°), and 26.6° now lands at 9.1078 against LOOP's 9.1063. Swept over aoa −30° to 60° at beta 0° and 15°, on 4 and 12 panels, all 52 points converge on both solvers, NONLIN with a residual at or below 2e-11 everywhere.

The two red checks, and why neither is this diff

git diff origin/main -- src/solver.jl lies entirely between src/solver.jl:913 and :1010, inside if solver.solver_type == NONLIN ... return nothing end. The LOOP branch starts at :1012, and solve_base! (src/solver.jl:689) only ever re-runs gamma_loop! for LOOP — it never switches solver type. No solver_type=LOOP solve can reach a changed line.

GitHub CI, Julia 1.12 · ubuntu · x64 · with code coverage, failed test/solver/test_forwarddiff.jl:88 — rel_err < 0.001 evaluated 0.0400 < 0.001 — on a solver_type=LOOP solver, with the other five matrix cells green on the same commit. Measured at that operating point on the box, the same quantity is 3.31e-6, repeatable to four significant figures and 300x inside the bound; CI's 0.0400 is 12000x that, so it is a discrete derivative jump near a polar knot, not tolerance drift. The repo has met it twice before, and 320e756 records the same jump at 0.044. I did not restore the operating point 320e756 chose against it, because I measured that it does not move: rel_err is 3.314e-6 at theta = 0 and the identical 3.314e-6 at 1e-12, 1e-9 and 1e-6 either side, 3.316e-6 at 0.25° and 3.273e-6 at 1.0°. theta_idxs feeds twist and linearize passes delta_angles=nothing, so that change never reached a delta knot; its green run came from the 0.1 bound the same commit set, which ae11d48 was right to tighten back. #287 carries the measurements, the two candidate knots (every panel's delta is exactly 0.0, exactly a knot of delta_range = -3:3:3; the tip panels solve to alpha 19.04° and 18.93°, outside alpha_range = -5:5:15) and the fix that would follow from the first.

agent ci-local reported Plotting (Makie) | 4 passed 1 failed 5 total. The fifth assertion in that testset is an isfile on Rectangular_wing_geometry_angled_view.png under save_dir = tempdir(), which is the shared /tmp; the several VortexStepMethod worktrees on this box run suites at once and the testset safe_rms those exact fixed names. The next full run on the identical commit passed the testset 58/58. #286 carries it, with the four other test files that write fixed names into the shared tempdir.

Neither is a reason to change this diff, and neither could be fixed here without guessing: I have left both to their issues.

The other half of the issue

The 3.0e-4 column above is rtol / relaxation_factor, flat across the sweep, and that is not a coincidence: LOOP converges on max|gamma_new - gamma|, which is relaxation_factor * (F(gamma) - gamma), so rtol = 1e-5 buys a residual of 3.3e-4 — and solve_base!'s retry at half the relaxation factor loosens it again. That is a real defect but it moves every LOOP reference number in the suite, so it is #284 rather than a rider here.

The testset the issue flagged, NONLIN solve! re-runs across calls, is untouched: it solves at exactly this operating point, which now converges, and the new testset asserts convergence there rather than leaving it implied.

Verification

  • Reproduced first: NONLIN vz=5.0 aoa=26.6 converged=false max|gamma|=8.7813 max|F-g|=5.17 against LOOP ... converged=true max|gamma|=9.1063 max|F-g|=0.00282
  • test/solver/test_solver.jl red before the change (FAILURE == FEASIBLE, and [3.335, 8.781, 8.781, 3.335] against [4.436, 9.106, 9.106, 4.436]), green after (juliaserver, 3/3)
  • Affected tests green: solver/{test_solver, test_flow_curvature, test_forwarddiff, test_unrefined_dist}.jl and body_aerodynamics/{test_body_aerodynamics, test_results}.jl — the NONLIN callers and the linearize/ForwardDiff paths around them
  • Local full suite on this commit: PASS — 6325 passed, 1 broken, 8m05s of tests, Julia 1.12.7 (agent ci-local, second run). The first run's FAIL on the same tree was the /tmp race in Tests write fixed filenames into the shared tempdir, so two suites on  #286.
  • Docs build clean (docs/make.jl, only the pre-existing page-size warnings) · up to date with main · REUSE lint n/a, the repo carries no REUSE setup · jetls check not run: jetls: command not found on this box
  • Benchmark: solve_base!, NONLIN, 12 panels — 1200 B before and after, at 11.3° and at 26.6°, with max|gamma| identical to six digits. Times 0.053 → 0.049 ms and 0.063 → 0.076 ms, but the box was at 49/62 GB with 28 GB of swap in use, so read the allocations and not the times.
  • Risk: the backtrack has a floor — after six halvings it takes the last trial whether or not it improved. That cannot fake convergence, but an operating point whose Jacobian is bad rather than whose step is long would crawl at 1/64 of a step to max_iterations and report FAILURE, as it does today.

Scope

+70 / −18 across 3 files: the NONLIN branch of gamma_loop!, one testset, one changelog entry. Unchanged this round — the round only diagnosed the two red checks. No stack. Opened #284 for the LOOP tolerance, #286 for the shared-tempdir test race, #287 for the test_forwarddiff.jl knot flake.

Closes #283 · task VortexStepMethod.jl-283

The Newton iteration took the full step every time, which past stall
overshoots into a limit cycle rather than converging. Take the largest of
1, 1/2 ... 1/64 of the step that brings the fixed-point residual down; the
accepted trial's residual is reused as the next iteration's, so an undamped
iteration costs exactly what it did before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NsZ96PpodRmKZ82iMJnnGf
@1-Bort-1 1-Bort-1 added agent:running Agent task state agent:ci Agent task state agent:queued Agent task state and removed agent:running Agent task state agent:ci Agent task state labels Sep 9, 2026
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@1-Bort-1

1-Bort-1 commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

Local full suite: PASS (32 min, Julia 1.12.7, one cell of the matrix)

@1-Bart-1
1-Bart-1 merged commit fb1eea7 into main Sep 10, 2026
21 of 23 checks passed
@1-Bart-1
1-Bart-1 deleted the agent/283-nonlin-misses-the-tolerances-at-26-6-whe branch September 10, 2026 06:47
@1-Bort-1 1-Bort-1 added agent:done Agent task state and removed agent:review Agent task state labels Sep 10, 2026
1-Bort-1 added a commit that referenced this pull request Sep 10, 2026
…ter-chord

Conflict: CHANGELOG.md — main's #285 NONLIN backtracking entry and this
branch's panel_axes entry both landed under ## Unreleased / ### Fixed.
Kept both bullets.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CfxpbLet2tXKWdNUi9azPS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:done Agent task state

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NONLIN misses the tolerances at 26.6° where LOOP converges

2 participants