Repository navigation
NONLIN misses the tolerances at 26.6° where LOOP converges - #285
Merged
1-Bart-1 merged 1 commit intoSep 10, 2026
Merged
Conversation
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
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced Sep 9, 2026
Member
Author
|
Local full suite: PASS (32 min, Julia 1.12.7, one cell of the matrix) |
This was referenced Sep 9, 2026
1-Bart-1
approved these changes
Sep 10, 2026
1-Bart-1
deleted the
agent/283-nonlin-misses-the-tolerances-at-26-6-whe
branch
September 10, 2026 06:47
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
This was referenced Sep 10, 2026
This was referenced Sep 11, 2026
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.
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 distributionLOOPfinds.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 residualmax|F(gamma) - gamma|each one actually stopped at:(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 inmax|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, withcond(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 neargamma = 9.1. The fix is the standard one — accept a step only if it reducesmax|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 againstLOOP'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,NONLINwith a residual at or below 2e-11 everywhere.The two red checks, and why neither is this diff
git diff origin/main -- src/solver.jllies entirely betweensrc/solver.jl:913and:1010, insideif solver.solver_type == NONLIN ... return nothing end. TheLOOPbranch starts at:1012, andsolve_base!(src/solver.jl:689) only ever re-runsgamma_loop!forLOOP— it never switches solver type. Nosolver_type=LOOPsolve can reach a changed line.GitHub CI,
Julia 1.12 · ubuntu · x64 · with code coverage, failedtest/solver/test_forwarddiff.jl:88—rel_err < 0.001evaluated0.0400 < 0.001— on asolver_type=LOOPsolver, with the other five matrix cells green on the same commit. Measured at that operating point on the box, the same quantity is3.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_erris 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_idxsfeeds twist andlinearizepassesdelta_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'sdeltais exactly 0.0, exactly a knot ofdelta_range = -3:3:3; the tip panels solve to alpha 19.04° and 18.93°, outsidealpha_range = -5:5:15) and the fix that would follow from the first.agent ci-localreportedPlotting (Makie) | 4 passed 1 failed 5 total. The fifth assertion in that testset is anisfileonRectangular_wing_geometry_angled_view.pngundersave_dir = tempdir(), which is the shared/tmp; the several VortexStepMethod worktrees on this box run suites at once and the testsetsafe_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:LOOPconverges onmax|gamma_new - gamma|, which isrelaxation_factor * (F(gamma) - gamma), sortol = 1e-5buys a residual of 3.3e-4 — andsolve_base!'s retry at half the relaxation factor loosens it again. That is a real defect but it moves everyLOOPreference 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
NONLIN vz=5.0 aoa=26.6 converged=false max|gamma|=8.7813 max|F-g|=5.17againstLOOP ... converged=true max|gamma|=9.1063 max|F-g|=0.00282test/solver/test_solver.jlred 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)solver/{test_solver, test_flow_curvature, test_forwarddiff, test_unrefined_dist}.jlandbody_aerodynamics/{test_body_aerodynamics, test_results}.jl— theNONLINcallers and thelinearize/ForwardDiff paths around themagent ci-local, second run). The first run's FAIL on the same tree was the/tmprace in Tests write fixed filenames into the shared tempdir, so two suites on #286.docs/make.jl, only the pre-existing page-size warnings) · up to date withmain· REUSE lint n/a, the repo carries no REUSE setup ·jetls checknot run:jetls: command not foundon this boxsolve_base!,NONLIN, 12 panels — 1200 B before and after, at 11.3° and at 26.6°, withmax|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.max_iterationsand reportFAILURE, as it does today.Scope
+70 / −18 across 3 files: the
NONLINbranch ofgamma_loop!, one testset, one changelog entry. Unchanged this round — the round only diagnosed the two red checks. No stack. Opened #284 for theLOOPtolerance, #286 for the shared-tempdir test race, #287 for thetest_forwarddiff.jlknot flake.Closes #283 · task
VortexStepMethod.jl-283