cleanup: chord_frame_coordinates should build its frame with panel_axe - #308
Open
1-Bort-1 wants to merge 5 commits into
Open
cleanup: chord_frame_coordinates should build its frame with panel_axe#3081-Bort-1 wants to merge 5 commits into
1-Bort-1 wants to merge 5 commits into
Conversation
chord_frame_coordinates hand-rolled a chord frame: the unleaned mid-chord axis, norm(chord_vec) for the chord, and a normal whose sign it took off panel.z_airf. None of the three matches the frame loft_contour_node lofts the contour nodes along, which is the frame its fractions are compared against, and the sign guard read the one thing a replayed log frame does not restore. It is now VortexStepMethod.panel_axes over the panel's corners, ordered along the wing's spanwise_direction. The order matters: reinit!(::Panel) swaps the sections of a span-flipped wing before storing the corners where read_aero_log_points! writes the structural order, so a frame built on the stored order alone turns over on replay. store_chord_weights! now takes its widths from the same corners (bitwise the same number) so one definition of the chord blend weight serves both orders. LivePolarState.control_fraction is deleted; it was written and never read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L5KwrDwM5aBsHpGxnk2jNG
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
Author
|
Local full suite: PASS (24 min, Julia 1.12.7, one cell of the matrix) |
private_functions.md renders over Documenter's 500 KiB hard threshold with the five new helpers listed; main's page passes. The split that makes room is #301. The helpers stay documented in source.
…rd-frame-coordinates-should-b
…age" This reverts commit 3888986.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vQP5S9NeTsQJuodta38Wg
6 tasks
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
chord_frame_coordinatesbuilt its own chord frame by hand, and it was not the frame the thing it is compared against is built in. It is nowVortexStepMethod.panel_axesover the panel's corners — ordered along the wing'sspanwise_direction, which turns out to matter, because a replayed log frame hands those corners back the other way round.What was wrong
The fraction
chord_frame_coordinatesreturns is matched against contour-node fractions inbuild_station_point_map!, andloft_contour_nodeplaces those nodes alongpanel.x_airfoverpanel.chord— thepanel_axesquantities. The hand-rolled frame measured along the unleaned mid-chord axis, overnorm(chord_vec), and took its normal's sign offpanel.z_airf. Three divergences from the frame it is compared to, and the third reads exactly the state a replay does not restore:read_aero_log_points!puts the corners back and nothing else.On a panel with a 2.45° chord lean (blend weight 0.62) and 1.7% taper between its section chords, a point at 40% chord and 3% chord off the chord line reads as (0.4077, 0.01307) through the old frame. The offset — the camber increment a live polar is regenerated from — is out by a factor of 2.3, because 0.4·sin(2.45°) = 0.017 chords of the along-chord displacement bleeds into the normal component, which is the same size as the 0.03 being measured. The new frame reads (0.4, 0.03) exactly.
Hand that panel the
z_airfa previous solve left pointing the other way, which is what a replay leaves, and the old answer's offset flips sign: +0.01307 → −0.01307. Through the real replay path on the fixture the same flip moves every control-point offset from +0.01587 to −0.01587.The thing I did not expect
Doing exactly what #302 describes —
panel_axesovercorner_pointswithorient = 1— is right for a live frame and wrong for a replayed one on a span-flipped wing, and the sign guard being removed is what used to absorb that.reinit!(::Panel)swaps the two sections of a span-flipped wing before storing its corners.write_live_aero_log_points!logs(sec_le[i], sec_te[i], sec_te[i+1], sec_le[i+1])— the structural order, always. So the corners a replay restores are the live ones with the columns reversed. Measured, by forcing the flip on the 2plate fixture and replaying:The old code was invariant to that reordering — reversing the corners flips
le_1 - le_2, hence the normal, and thepanel.z_airfguard put it back — at the cost of reading state a replay leaves stale. Removing the guard without replacing it would have turned the chord frame over on exactly the path the issue set out to make correct.So the frame is built on the corners ordered along the wing's
spanwise_direction(spanwise_corners). That is static configuration, not solve state, so it survives a replay; and in the live case it is a no-op, soorient = 1stays right, as #302 says. The chord blend weight follows the same order:panel_chord_weight(wp, wo, wn) = 1 − panel_chord_weight(wn, wo, wp), so walking the neighbours in the direction the corners run gives the complement where they run against the panel index.What changed
chord_frame_coordinates(panel, spanwise, chord_weight, pos_b)is four span-ordered corners intopanel_axeswithorient = 1; the sign guard and the degenerate-normal fallback go with the cross product they guarded.store_chord_weights!now takes its widths frompanel_span_vectorover the corners instead ofpanel.width. That is the same number to the bit —panel.widthissmooth_norm(bound_point_2 - bound_point_1)over the same0.75·le + 0.25·tepoints, and I checked bitwise equality on every panel of the fixture — so the symbolic path is untouched, and onechord_blend_weightsnow serves both orders.LivePolarState.control_fractionis deleted. It was written and never read; the@warnthat looked like a reader was measuring the station groups' own lengths, so it measures them directly now.The divisor does not wash out
#302 asks that the change of divisor —
norm(chord_vec)topanel_chord— be established rather than assumed to wash out through the reference differencing andchord_residual!. Established, on the same swept, tapered panel: three control points on the chord line at 10/50/90%, bumped 2% of chord off it at mid-chord, run throughcontrol_point_deflectionandchord_residual!in three frames.The lean does wash out of the deflection, and for the reason the issue gives: the reference point and the deformed point are measured in the same frame at the same station, so the along-chord bleed is common to both and cancels in the difference — old against mix is 1.5e-6 relative. The divisor does not. It scales the deflection ordinates and stretches the chord stations they sit on by the same 1.7%, and neither of those is the constant-or-affine-in-x part
chord_residual!removes: the camber residual the polar is regenerated from moves 3.8%, 0.02010 → 0.01933.The fraction is worse off still, because at the
build_station_point_map!call site there is no differencing at all — it is matched againstxc[k]raw, and it was reading 0.1016 for a node at 0.1.Where I would push back
Nothing in this repo's fixtures moves. The 2plate kite's sections are parallel and equally spaced, so every chord blend weight is exactly 0.5,
norm(chord_vec)equalspanel_chord, and every control-point frame comes out within 1e-11 of the old one. The defect is real but latent: it needs a swept, tapered or cosine-distributed mesh, and this package has no such test wing. That is why the test that proves the change is a hand-built panel, and why I would not claim this fixes a number anyone has seen.The same gap bounds the replay test. That fixture's stations name only their chord ends — the very points the logged corners are reconstructed from — so a replayed control point sits exactly on the chord line and its offset is 0. A pipeline test of "the frame reads nothing but the corners" is zero against zero there. So the replay testset holds what the fixture can show (the logged corners land, the live shape comes back agreeing), and the span-flip regression is caught by an exact unit assertion instead: the same panel with its corners in the other section order must give the same answer.
Four more things found on the way, all out of scope and unfixed here:
panel_chord_width(src/aero_modes/common.jl:472) andfreeze_traction_pattern!(pressure.jl:749) are two more hand-written copies of "the panel's chord frame and size" — the same family cleanup: the two frames #284 left hand-written #300 is about. Said on cleanup: the two frames #284 left hand-written #300 rather than opened again, and cleanup: the two frames #284 left hand-written #284 moved `panel_forc #306 has since takenpanel_chord_width.main, and both add the same equation tosrc/aero_modes/common.jlunder names one!apart:chord_blend_weights!(chord_weight, width)there,chord_blend_weights(width, step)here. Mine is the general one —step = 1is exactly theirs,step = -1is the spanwise corner order this change needs — so whoever merges second should delete the in-place copy and writechord_weight .= chord_blend_weights(width, 1), which also givesstore_chord_weights!its allocation back. Neither branch can stack on the other now: both are pushed, andagent stackonly moves a branch the remote does not have. Said on cleanup: the two frames #284 left hand-written #284 moved `panel_forc #306 too..github/workflows/CI.yml:38carriesfail-fast: false, which rules/AGENTS.md §4.10 wants gone — cleanup: CI's test matrix carries fail-fast: false #307.SysState(n)defaults toKiteUtils.MyFloat, which isFloat32, so a log round trip moves the panel corners by ~8e-7 and the deflection by ~5.8e-7. The replay test logs atprecision = Float64. Worth knowing before anyone asserts tightly on replayed geometry.Verification
(0.4077450372755361, 0.013073955934775493), and(0.4077450372755361, -0.013073955934775493)oncez_airfis the one a previous solve left. Through the real replay path on the fixture every control-point offset moves+0.015868246442849→-0.015868246442850; with the new frame it does not move at alltest/test_live_polar.jlred before, green after — the new assertions run against a verbatim copy of the old function in.agent/scratch/red_check.jl, whose output is the numbers above; 159 pass after (juliaserver, exit 0)test/test_pressure_aero.jl80 + 20 pass ·test/test_continuous_aero.jl52 pass ·test/test_aero_modes.jl779 pass ·test/test_backend_parity.jl3 passorigin/main(d6df9d93), and every test above re-run on it, which is VortexStepMethod 5.panel_axes,panel_span_vector,panel_chord_weightandsmooth_normare unchanged from 4.3.1e5caca93(run 34360750500) · Local full suite: PASS (32 min, one matrix cell). Both ran on the merged head: the commit lands 14:00:11 UTC, the CI run opens 14:00:24 andagent ci-localstarts 14:00:20docssub-project is not installed). The five private helpers are listed in the@docsblock ofdocs/src/private_functions.md, which Documenter'smissing_docscheck requires of every docstring in the packagereuseis not installed on this box, so./bin/reuse_lintdoes not run here)corner_chord_weightsis oneVectorand O(n_panels) of arithmetic perupdate_live_deflection!, against a NeuralFoil pass in the same call.store_chord_weights!was allocation-free and now allocates twon_panelsvectors per circulation refresh; unmeasured, and the shape below is where that goes awayspanwise_cornersdecides the order ondot(span, spanwise) < 0. A wing bent past 90° between its tips would have panels disagreeing about which way+spanwiseis, and would get a frame that turns over mid-span.panel_span_signsalready makes the same per-panel assumption, so this is not new, but it is the line I would look at first.Scope
+173 / −55 across 6 files.
src/aero_modes/{common,live_polar,pressure}.jlis the change;test/test_live_polar.jlis two new testsets and the frame arguments threaded through the existing one. No stack.Closes #302 · task
SymbolicAWEModels.jl-302