Skip to content

Take the panel normal from the quarter-chord step - #273

Open
1-Bart-1 wants to merge 5 commits into
mainfrom
fix/panel-normal-quarter-chord
Open

Take the panel normal from the quarter-chord step#273
1-Bart-1 wants to merge 5 commits into
mainfrom
fix/panel-normal-quarter-chord

Conversation

@1-Bart-1

@1-Bart-1 1-Bart-1 commented Sep 5, 2026

Copy link
Copy Markdown
Member

Fixes #272.

panel_axes built the panel normal from the leading-edge step:

span_vec = panel_span_vector(le_1, te_1, le_2, te_2)   # quarter-chord step
y_airf   = orient .* (span_vec ./ width)
z_cross  = cross(x_airf, le_1 .- le_2)                 # leading-edge step

The bound vortex, width and y_airf all come from the quarter-chord step, so
z_airf was the odd one out. On a panel whose two sections have
differently-directed chords the normal is then not square to the vortex the loads
are built from, and since alpha = atan(v·z_airf, v·x_airf) feeds every polar
lookup, the error propagates into cl, cd and cm for that panel.

One character of the fix: cross(x_airf, span_vec). The frame now closes as
z_airf = x_airf × y_airf, which is what the Panel field docstring
(src/panel.jl:28) has always claimed.

Why it went unnoticed

Taper alone cannot trigger it. (le_1 - le_2) - span_vec equals
¼(chord_vec_2 - chord_vec_1), which is purely chordwise when the two chords are
parallel, and cross(x_airf, ·) annihilates the chordwise part exactly. It takes
twist, sweep or dihedral — something that changes the chord direction between
the two sections — for the normals to separate at all.

The reference wings barely do that:

reference wing angle(LE step, QC step) normal changes?
:rectangular 0.84°, from its ±0.5 rad twist yes
:curved 0.00° — LE and TE share (y, z) no
:elliptical 34.01° no — planar, so the normal is ±ẑ regardless

The elliptical wing separates the two steps by 34° and still cannot see the
difference. Nothing in the suite combines sweep with a changing chord direction,
which is the only regime where the choice matters.

Changes to the reference oracle

test/thesis_oriol_cayon.jl makes the same mix (its x_airf is the normal, its
z_airf the spanwise axis):

x_airf = cross(VSMpoint - LLpoint, section["p2"] - section["p1"])  # LE step
z_airf = bound["x2"] - bound["x1"]                                 # QC step

so the inconsistency is inherited from the original formulation rather than
introduced by the port, and the oracle has to move with the code or it pins the
old convention. It now takes the normal from the bound filament it already builds
— a two-line change that reuses bound["x2"] - bound["x1"] instead of recomputing
from the leading edges.

This is the part worth reviewing carefully, since it is the test changing to
match the code. It is defensible here because the oracle's own spanwise axis is
already the quarter-chord step, so the change makes it self-consistent rather than
merely agreeing with us. Without it, 156 assertions fail (2 models × 39 panels × 2
asserts, all on :rectangular) at ~0.25° — real but small.

To avoid leaning on the oracle for the thing being fixed, the new test in
test/panel/test_panel.jl states the invariant directly on a swept, twisted panel
where the two definitions are 13.7° apart: the normal is square to the bound
vortex and to the chord, the frame closes as z = x × y, and orient flips y
and z together. It carries a guard asserting the geometry actually discriminates
the two definitions, so it cannot silently decay into a test of nothing.

Measured effect

SK100 leading-edge-inflatable kite, 44 panels, outermost rib bay raked 58–68°,
where the old and new normals are 5.6–13.8° apart. Coupled beam + VSM flight,
identical settle (1200/1200 steps):

z_cross from steps flown before the VSM solve stops converging
le_1 - le_2 122
span_vec 189

Same failure mode either way — a tip-stall divergence — so the frame error
aggravates it rather than causing it, but it is worth ~55% more flight time here.

Deliberately not included

Orthogonalising y_airf against x_airf. It measures better on the kite (217
steps), but that metric records how long a stall divergence is deferred, not
accuracy, and both of y_airf's uses argue for keeping the raw quarter-chord
direction: dir_lift is Kutta–Joukowski (F = ρ v × Γ, Γ along the bound
vortex), and q_dyn = ½ρ|v × y|² is the simple-sweep normal-to-quarter-chord
component. Settling that needs a swept, tapered validation case with a known
answer, which does not exist in the suite yet. Left open on #272.

Downstream

SymbolicAWEModels.jl rebuilds this same expression symbolically in
build_panel_force_eqs and again in wagner_reference_frame. Those need the
matching change or its symbolic force assembly disagrees with this solver.

🤖 Generated with Claude Code

panel_axes built z_airf as cross(x_airf, le_1 - le_2) while the bound
vortex, width and y_airf all come from panel_span_vector. On a panel whose
two sections have differently-directed chords the normal was not square to
the vortex the loads are built from, and alpha is measured against that
normal, so every panel coefficient inherited the error. The frame now closes
as z_airf = x_airf x y_airf, which is what the Panel field docstring already
described.

Taper alone never triggered it: (le_1 - le_2) - span_vec is a quarter of
chord_vec_2 - chord_vec_1, purely chordwise when the two chords are parallel,
and cross(x_airf, .) annihilates it. Twist, sweep and dihedral do trigger it.

The reference implementation makes the same mix, so it moves with the code:
its normal now comes off the bound filament it already builds. Its three
wings barely exercise the difference - the elliptical one separates the two
steps by 34 degrees and still cannot see it, being planar - so the new panel
test states the invariant directly on a swept, twisted panel where the two
definitions are 13.7 degrees apart.

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

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Comment thread src/panel_aerodynamics.jl Outdated
Airfoil frame and size of the panel between two sections, as
`(; x_airf, y_airf, z_airf, chord, width)`.

`x_airf` is chordwise and `y_airf` runs along the quarter-chord line the bound

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.

@1-Bort-1 rewrite this functions docstring

@1-Bort-1

1-Bort-1 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Picked up as VortexStepMethod.jl-273, on the branch this pr already has.

@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 6, 2026
@1-Bort-1 1-Bort-1 added agent:review Agent task state and removed agent:running Agent task state labels Sep 6, 2026

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

Copy link
Copy Markdown
Contributor

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 · 0 inline, 0 off the diff

Good

  • The one-line fix cross(x_airf, span_vec) is mathematically sound: z_airf = orient·(x×y) exactly equals the documented x_airf × y_airf (normalised), verified against panel.jl:28 and panel_span_vector (quarter-chord step).
  • The docstring rewrite removes the stale chord_weight rationale (a "why") and states what each axis is, matching rubric §4; the x_airf × y_airf normalised claim is exactly what the code computes.
  • The new test states the invariant independently of the oracle — square to vortex/chord, z = x×y closure, orient flips — plus a guard (separation > 5) so it cannot decay into testing nothing.

Not good

  • The matching build_panel_force_eqs/wagner_reference_frame change in SymbolicAWEModels.jl is deferred with no linked issue/task, so the two repos risk silently diverging on z_airf.
  • The orient test passes chord_weight=0.5 positionally (line 202), burying a magic number that duplicates the signature default.

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 1-Bort-1 added agent:queued Agent task state agent:running Agent task state and removed agent:review Agent task state agent:queued Agent task state labels Sep 7, 2026
Both sides added to the Unreleased changelog section; kept both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rz8GuEDbg9WMJVZPUYvt2D
@1-Bort-1 1-Bort-1 added agent:review Agent task state and removed agent:running Agent task state labels Sep 7, 2026
This was referenced Sep 7, 2026
@1-Bort-1 1-Bort-1 added agent:queued Agent task state agent:running Agent task state and removed agent:review Agent task state agent:queued Agent task state labels Sep 7, 2026
@1-Bort-1

1-Bort-1 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

The branch had stopped merging into main again. #282 landed there — solve!(...; throw_on_fail=true), SolveFailure and finite_full — and opened a fresh ## Unreleased section in CHANGELOG.md with an ### Added under it, where this branch already had an ### Fixed under a heading of the same name. That heading was the whole conflict; nothing else in the merge overlapped.

Resolved by keeping both entries under the one ## Unreleased, ### Added before ### Fixed, which is the order the released sections in that file already use. Neither side was taken wholesale and neither entry was reworded.

The merge brings no code near this fix: #282 is confined to src/solver.jl, src/VortexStepMethod.jl, the two docs pages and test/solver/test_solver.jl, none of which this branch touches. git diff origin/main --stat is still the same four files, +54/-11 — src/panel_aerodynamics.jl, test/panel/test_panel.jl, test/thesis_oriol_cayon.jl and CHANGELOG.md.

One correction to what an earlier round said here: the oracle test/thesis_oriol_cayon.jl is reached only from test/body_aerodynamics/test_body_aerodynamics.jl, not from the solver suite. The solver suite is in the list below because the merge changed it, not because the oracle change touches it.

If you are re-running any of this locally: test/solver/test_solver.jl now needs ForwardDiff, which the examples environment does not carry, so it wants Pkg.activate(".").

Verification

  • test/panel/test_panel.jl on the merged tree: 24 + 8 pass, exit 0
  • test/body_aerodynamics/test_body_aerodynamics.jl: all pass, including the 4803-assertion Wing Geometry Creation that compares against the oracle
  • test/solver/test_solver.jl: all 9 testsets pass, including main's new solve! reports a solve that missed the tolerances and finite_full sees a Dual's partials, not just its value
  • Risk: none from this turn — the resolution is two changelog entries under one heading, and the code on both sides of the merge is disjoint.

Commits

  • 536a67f Trace the shrink wrap's rolling ball exactly instead of on a grid
  • db00f13 Describe the rolling-ball wrap where the distance field was documented
  • 8aa6ced Give a wing's mesh and airfoil settings a home in the settings file
  • 53a7e50 Carry table_format as the Symbol the generator takes
  • 320e756 Make the POLAR_MATRICES forwarddiff check robust on Windows
  • ae11d48 Make polar generation deterministic so the forwarddiff test stops flaking on Windows
  • 9b5edcc Plumb n_bins through obj_to_yaml and trim the settings docstrings
  • 0694a84 Read the mesh and airfoil blocks with convertdict
  • 3afe670 Merge remote-tracking branch 'origin/main' into wrap/rolling-ball
  • ecc852c Keep a closed loop's trailing edge sharp at zero clearance
  • 1e13d92 Offset the rolling ball's contact polygon instead of its rim
  • 10ecca3 Name the sliced mesh in the wing's mesh block
  • a80b8c2 Give the settings blocks the defaults their callees already apply
  • 39d757f Say what the panel frame is, not why it is that way
  • 257d084 Merge pull request Continuous wrapping method #262 from OpenSourceAWE/wrap/rolling-ball
  • eeb1f3e Merge branch 'main' into feat/aero-settings
  • 0a8f046 Merge pull request Give a wing's mesh and airfoil settings a home in the settings file #271 from OpenSourceAWE/feat/aero-settings
  • 81bb853 Merge branch 'main' into fix/panel-normal-quarter-chord
  • 74c72cf Release v4.4.0
  • 5612d24 Release v5.0.0 instead
  • c90ece8 Merge pull request Release v5.0.0 #281 from OpenSourceAWE/agent/276-release-v4-4-0
  • 0ef6b53 Merge remote-tracking branch 'origin/main' into fix/panel-normal-quarter-chord
  • 544badd Let a caller ask solve! to throw on a failed solve
  • 6425883 Cover finite_full and the converged path of throw_on_fail
  • 3f75701 Merge pull request Let a caller ask solve! to throw on a failed solve #282 from OpenSourceAWE/agent/SymbolicAWEModels.jl-290
  • 58583fb Merge remote-tracking branch 'origin/main' into fix/panel-normal-quarter-chord

Review just these changes

Task VortexStepMethod.jl-273

@1-Bort-1 1-Bort-1 added agent:ci Agent task state agent:review Agent task state and removed agent:running Agent task state agent:ci Agent task state labels Sep 9, 2026
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.

Panel normal z_airf is built from the leading-edge step, not the quarter-chord step the bound vortex uses

2 participants