Skip to content

Default correct_aoa to true, with the aero-centre flow leaving out only the section's own bound vortex - #400

Draft
1-Bort-1 wants to merge 10 commits into
agent/94-remove-results-dictfrom
agent/318-correct-aoa-defaults-to-false-so-the-sol
Draft

1-Bort-1 wants to merge 10 commits into
agent/94-remove-results-dictfrom
agent/318-correct-aoa-defaults-to-false-so-the-sol

Conversation

@1-Bort-1

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

Copy link
Copy Markdown
Member

TL;DR

The solver now turns each section force by the flow at its 1/4-chord aerodynamic centre while still reading the polars at the 3/4-chord control point (Gaunaa, Li & Pirrung 2026, TAT2 + TAT3), and that flow now leaves out only the panel's own bound filament (their Eq. 2), not every panel's. With the old default the elliptic AR 4 wing reached an Oswald efficiency of 0.73 against the theoretical 1. It now reaches 0.998.

What was wrong

With correct_aoa=false, one velocity both read the polars and turned the force, which the paper calls LL-3/4. That keeps lift but loses induced drag. correct_aoa=true already did TAT2 + TAT3, but its aero-centre matrix zeroed every panel's bound filament (evaluation_point_on_bound was a global flag), which is a stronger removal than the "undisturbed" flow of TAT3. On a straight wing that makes no difference, because the other bound filaments are colinear with the evaluation point and induce nothing there. On a swept or curved wing it does.

Measured on this branch: VSM, INVISCID (Cl = 2πα), α = 10°, default core radius, e = CL² / (π · AR_projected · CD):

wing old default (false) true, every bound filament zeroed true, Eq. 2 (this PR)
straight elliptic AR 4 (41 sections) 0.728 0.998 0.998
straight elliptic AR 20 (121 sections) 0.879 1.001 1.001
elliptic AR 4, sweep 0.2·|y|, anhedral 0.5·|y| (121 sections) 0.821 1.952 1.110

The regression test runs the AR 4 case through the Python verification settings, core_radius_fraction = 1e-20: there it gives e = 0.729 with false and 0.999 with true (CL 0.6498 → 0.6505), and it asserts e within 0.01 of 1 for true and below 0.8 for false.

The middle column shows why the Eq. 2 part matters: on the swept, anhedral wing, zeroing every bound filament halves the induced drag (e = 1.95). CL moves by less than 0.2 % throughout. e > 1 on the non-planar wing is expected, since e is taken on projected span. I have no VLM reference for that case, so 1.110 is not checked against an independent number.

On the V3 kite

TU Delft V3 kite, VSM, alpha 0-24 deg: CL and CD with correct_aoa=false (old default) and true (new), against wind tunnel and RANS

CL and the stall angle (14°) do not move, because the polars are still read at 3/4 chord. CD drops most around the L/D peak: at α = 10° it goes from 0.0998 to 0.0742, and max L/D rises from 9.28 to 12.13. The new curve sits on the RANS drag, and the old one sat closer to the wind tunnel. Where I would push back on my own result: the wind-tunnel number is the one a flight simulator is tuned against. Anyone who calibrated to the old drag will see about 25 % less of it, and correct_aoa: false gets the old curve back. The "before" is the same branch with correct_aoa=false. With that setting the aero-centre matrix is never built, so it runs exactly the pre-PR code path.

Eq. 2 also moves LLT on curved wings, which builds its whole solve on the aero-centre matrix: V3 kite at α = 10°, LLT CL goes from 1.070 to 0.905, now close to VSM's 0.894.

What the change turned up

  • calc_forces! stopped being allocation-free (336 B/call, caught by calc_forces! is zero-alloc). calculate_AIC_matrices! built two temporary arrays and two escaping MVectors, which did not matter while it ran once per solve but did once calc_forces! called it every time. They now come from body_aero.cache[2:3] and work_vectors[6:9], and the stale # 64 bytes note in calc_forces! is gone.
  • ForwardDiff lost a derivative on the vortex axis. A refined straight segment puts each aero centre exactly on its neighbours' bound axes. There the velocity is zero but linear in the off-axis distance, and the kernel either returned a hard zero or normalised a zero radial vector. test_forwarddiff.jl's POLAR_MATRICES case went from 3e-8 to 1.5e-4 relative error, and FiniteDiff agreed with itself at steps 1e-8 to 1e-5 (−0.107626 vs AD −0.107892), so AD was the one that was wrong. The core branch is now the same velocity in closed form, −Γ/(4π) · d_sum/(ε² ℓ²) · (r1 × r0), which agrees with the old branch to 2e-11 on 10⁴ random points and stays smooth through the axis.
  • NaN with a near-zero core. The same aero centres, with core_radius_fraction = 1e-20 (as the Python verification cases use), fell through the colinearity test on rounding noise and came back NaN. A zero-core point within rounding of the axis now gets zero. The semi-infinite trailing kernel has the same shape, but its core is not linear in the distance, so the same fix would change its model. No point the solver builds reaches it, so it is velocity_3D_trailing_vortex_semiinfinite! returns NaN on its own axis at its start point, and its core is not linear in the distance #399 rather than part of this PR.
  • test_polar_vectors.jl compared cl_dist against alpha_dist, but under correct_aoa that is the aero-centre angle, and the polars are read at alpha_uncorrected. The test now names the angle the polars were read at. That was a test bug, and nothing in it was loosened: both fields are identical with correct_aoa=false.
  • test_stability.jl's trim tests checked |CMy| < 1e-5 at a trim bisected to α within 1e-5 rad. With dCMy/dα = 3.34 that bounds |CMy| by 3.3e-5, so the old pass (4.3e-6) was down to where the bisection landed, and the new default's 1.15e-5 failed. Both trims now bisect to alpha_tol=1e-7. The 1e-5 assertion is unchanged, and it now holds by construction.
  • vsm_settings_coarse.yaml carried correct_aoa twice, commented true and live false. It now has one live line, and that line is true, so anyone loading that file also gets the new behaviour.
  • Around the change: comments that restated code in calculate_AIC_matrices!, update_effective_angle_of_attack! and calc_forces! are gone, and the four uniform-inflow AIC calls in the matrix tests share one helper.

From the review, not done: the "LLT matrix leaves out only…" test builds its expectation from the filament kernels. It checks which filaments the matrix leaves out, not the kernel values, and those are pinned on their own in test_bound_filament.jl. A hand-computed off-diagonal entry would mean rewriting the test, so it is not part of this PR.

AR 4 at 121 sections does not converge with LOOP under either setting (NaN, FAILURE), so the AR 4 numbers use 41 sections. That is not this PR's doing. It looks like the same family as #395 and #315.

Verification

  • Reproduced first: straight elliptic AR 4, old default, e = 0.7283 (CL 0.64978, CD 0.046090)
  • test/verification/test_verification.jl "elliptic AR 4 wing reaches Oswald efficiency 1": red with the old default (22/23), green (23/23); it now also pins correct_aoa=false (LL-3/4) below e = 0.8, green 24/24
  • test/body_aerodynamics/test_body_aerodynamics.jl "LLT matrix leaves out only each panel's own bound filament": red on the old flag (9/15), green 15/15. "LLT matrix is finite on a swept wing without vortex core": red (NaN), green, and it now also matches the same matrix built with a 1e-10 core
  • test/filament/test_bound_filament.jl "ForwardDiff sees the core's slope on the extended axis": red (slope 0.0 vs 0.1414), green 227/227
  • Affected files green under juliaserver (test env): body_aerodynamics ×2, filament ×2, panel, settings, wake, and solver/{solver, flow_curvature, viscous_drag_correction, attached_trailed_force, moment_units, forwarddiff, polar_vectors, backend_comparison, wing_directions, unrefined_dist, stability}, verification
  • Merged the updated solve! fills VSMSolution with everything solve() returned; solve() and calculate_results are removed #392 branch (it now carries Measure alpha in the plane square to the bound vortex (Gaunaa CP1) #393's inflow_angle): one conflict in update_effective_angle_of_attack!, resolved to Measure alpha in the plane square to the bound vortex (Gaunaa CP1) #393's inflow_angle on this branch's own-bound-only matrix. Measure alpha in the plane square to the bound vortex (Gaunaa CP1) #393 had also trimmed BodyAerodynamics.cache to one buffer, under this branch's cache[5:6], so it holds three again. After the merge: body_aerodynamics ×2, filament bound, solver/{solver, polar_vectors, stability, forwarddiff}, verification all green (5146 assertions, calc_forces! is zero-alloc and the AR 4 Oswald test included)
  • Local full suite on 602fcd5: PASS (9 min, Julia 1.13.0, fail-fast) · GitHub CI: Documentation PASS; the test matrix is skipped while this PR is a draft, which it stays until solve! fills VSMSolution with everything solve() returned; solve() and calculate_results are removed #392 merges
  • After review: test_verification 24/24, test_body_aerodynamics green, test_solver green (calc_forces! is zero-alloc included), test_forwarddiff 10/10
  • Benchmark: inconclusive. Of three interleaved solve! runs on the V3 kite (false → true → false), two were discarded by bench-end (conditions moved, up to 22 of 20 cores busy). The one it kept gave 1.94×, but the same setting timed at either 3.3 ms or 6.7 ms between runs, so I'm not quoting a ratio. Allocations are deterministic: 6 allocations / 208 B per solve! with either setting. The added work is one more N×N×3 aero-centre matrix per solve!, which false skips.
  • Docs build: not run locally (no docs manifest here; CI builds it). No new public symbols.
  • Risk: users calibrated against the old drag level. Every VSM drag number moves, which is the point, but it is a behaviour change for anyone comparing to old runs.

Scope

+168 / −91 across 12 files. Stacked on #392, which moves correct_aoa handling into the solver and removes calculate_results. This PR targets its branch and merges after it. src/ holds the two defaults and their docstrings, the one-line Eq. 2 flag, the allocation fix, the closed-form core, and the comment trims. The rest is new tests, the test_polar_vectors.jl and test_stability.jl fixes, the yaml collapse and the changelog.

Opened by 1-Bort-1, an AI agent working for @1-Bart-1.
Closes #318 · task VortexStepMethod.jl-318

1-Bort-1 and others added 7 commits September 30, 2026 10:07
…vortex at the aero centre

Bart decided on #318 to default correct_aoa to true. The aero-centre
matrix used for the force directions now keeps every other panel's bound
filament (Gaunaa, Li & Pirrung 2026, Eq. 2) instead of zeroing all of them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… bound filaments

Also list correct_aoa in the Solver docstring.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ite on its own axis

The correct_aoa default now calls calculate_AIC_matrices! from calc_forces!
on every solve, so its temporaries come from the body's buffers. With a
near-zero core, a point on a filament's line fell through the colinearity
test on rounding noise and came back NaN.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the axis

With the aero-centre matrix keeping neighbouring bound filaments, an aero
centre on a refined straight segment lies on its neighbours' axes. The core
branch normalised the radial vector there and the old guard returned a hard
zero, so ForwardDiff lost the slope FiniteDiff sees. The closed form is the
same velocity (2e-11 relative) and linear through the axis.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… bound

cl_dist is read at alpha_uncorrected, which equals alpha_dist only without
correct_aoa. trim_angle's default alpha_tol bounds |CMy| by 3.3e-5 on the
trimmable wing, so the 1e-5 check passed by where the bisection landed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop comments that restate code, name the kernels' work-vector slots
correctly, share the uniform-inflow AIC call across the matrix tests and
hoist the ForwardDiff probe out of its testset.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@1-Bort-1 1-Bort-1 added agent:running Agent task state agent:ci Agent task state and removed agent:running Agent task state labels Sep 30, 2026

@1-Bort-1 1-Bort-1 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent review (advisory)

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

Good

  • Matches the plan: the default flips in both SolverSettings and Solver, and the Eq. 2 fix is the one-line model == LLT && icp == jring change at body_aerodynamics.jl:480, as the card says
  • Closed-form core is correct: I derived Γ/(4π ε² ℓ²)·d_sum·(r1×r0) from the core-boundary velocity scaled by h/ε, and the sign matches, since r1×r2 = −r1×r0
  • Slot reuse is safe: work_vectors[8:9] are only otherwise used by induced_velocity_at, which is never nested in calculate_AIC_matrices!, and the filament kernels only touch slots 1–5
  • body_aero.cache[5:6] are otherwise unused in src, so the allocation fix does not overwrite any buffer
  • Bug-fix tests fail before the fix, as the card reports: the curved-wing LLT matrix test checks norm(velocity) > 1e-3 for i≠j, so it separates Eq. 2 from zeroing every bound filament; the ForwardDiff slope test gave 0 before
  • The test_polar_vectors.jl and test_stability.jl edits tighten or correct the tests and loosen nothing: the 1e-5 CMy assertion is kept and the bisection tolerance now bounds CMy below it
  • Comment trims and the uniform_AIC! helper are behaviour-preserving cleanup around the change, and the card names them

Not good

  • src/settings.jl:202 — With true as the default, every VSM solve! (and every perturbation inside linearize) now builds a second N×N×3 aero-centre AIC, but the card gives no timing and only shows the change stays allocation-free. Simulators calling solve! every step will pay a cost nobody measured.
  • src/filament.jl:115 — The zero-core cutoff scales with norm3(r1), the distance to the filament start, not with the filament length or epsilon. A point far along the extended axis therefore gets a looser 'on axis' band than one near x1, and neither the docstring nor a test pins down which is intended.
  • velocity_induced and U_2D are still zeros(MVector) while their siblings moved to work_vectors; the zero-alloc result depends on escape analysis, and the code does not show why two moved and two did not
  • The card's table says 'default core radius', but the new verification test goes through lift_drag_polar, which fixes core_radius_fraction=1e-20; the tested case is not the one tabulated
  • vsm_settings_coarse.yaml changes its live value from false to true; the card calls this collapsing a duplicate, not a behaviour flip for users of that file
  • Now that false is no longer the default, no test pins the correct_aoa=false path (LL-3/4); anyone who sets it back to get the old drag has no regression guard
  • The 'LLT matrix leaves out only…' test rebuilds the AIC from the filament kernels, which is close to the parity-test pattern §2 warns about; a hand-computed expectation for one off-diagonal entry would be independent of the kernels
  • The swept-wing no-core test only asserts isfinite; it would also pass if the fallback zeroed entries it should not

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

Comment thread src/settings.jl
Comment thread src/filament.jl
@1-Bort-1 1-Bort-1 added agent:queued Agent task state agent:running Agent task state and removed agent:ci Agent task state agent:queued Agent task state labels Sep 30, 2026
…e zero-core fallback

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@1-Bort-1

1-Bort-1 commented Sep 30, 2026 •

Copy link
Copy Markdown
Member Author

Local full suite: PASS (10 min, Julia 1.13.0, one cell of the matrix)

@1-Bort-1 1-Bort-1 added agent:queued Agent task state agent:running Agent task state agent:ci Agent task state agent:review Agent task state and removed agent:running Agent task state agent:queued Agent task state agent:ci Agent task state agent:review Agent task state labels Sep 30, 2026
@1-Bort-1 1-Bort-1 added agent:queued Agent task state agent:running Agent task state and removed agent:queued Agent task state labels Sep 30, 2026
1-Bort-1 and others added 2 commits September 30, 2026 14:01
Takes #393's inflow_angle in update_effective_angle_of_attack! and keeps this
branch's own-bound-only aero-centre matrix; BodyAerodynamics.cache holds the
three buffers still read (induced velocity, panel areas, AIC inflow).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plotting changes only; no overlap with this branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:review Agent task state

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant