Spread sliced sections evenly over the span, not over leading-edge arc length - #350
Conversation
…c length station_indices now places its targets along the quarter-chord line with the chordwise component dropped, so a tip whose leading edge runs aft no longer gathers sections, and wingtip_distance is a spanwise inset. march_edges drops its arclen field, which only station_indices read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1-Bort-1
left a comment
There was a problem hiding this comment.
Independent review (advisory)
Verdict: APPROVE WITH COMMENTS · 1 inline, 0 off the diff
Good
- The fix matches the card:
station_indicesnow spreads targets over the quarter-chordhypot(Δy, Δz)length, and themin_chord_fractrim andwingtip_distanceuse that same length (obj_slice.jl:464-473). - Removing
arclenfrommarch_edgesis safe: a repo-widerg arclenfinds no other reader, andplot_slices_3din the Makie extension only callsstation_indices. - The docs are consistent: settings.jl, settings.md, airfoil_pipeline.md, both docstrings and the changelog all call
wingtip_distancea spanwise length [m], andrg 'arc length'finds nothing stale in the slicer path. - The new test checks both even spacing and the size of the inset; I worked it through by hand: all 61 stations stay above
min_chord_frac, the targets land within one 0.05 m station of the expected spacing, and the tolerance holds. - The change stays in scope: the only tidy-up is rewrapping the
obj_to_yamldocstring, as the card says, andmin_chord_fracpassthrough and #272/#273 are left out on purpose.
Not good
src/obj_adapter/obj_slice.jl:468— The card says this matches whatrefine_mesh_for_linear_cosine_distribution!measures, but that function (andcompute_refined_section_interpolation!) uses full 3D quarter-chord length withxincluded. The repo now has two different ideas of span position, and refinement still places panels by the length this PR stops using. Either correct the card and say so, or explain why the two may differ.- The test has z = 0 everywhere, so it only checks that
xis dropped. Changing the code to use plainywould still pass, even though the card's reason for includingzis the curled ram-air tip. - If closed tip stations step only in
x, theirspanvalues equalspan[first(usable)].argminthen returns the first of the tied indices, which is a closed station, and that silently undoes themin_chord_fractrim on the left tip. This is unlikely on real meshes but worth a line in the card. - The
station_indicesdocstring is 6 lines, above the rubric's 1-4. Its last two sentences could be cut down to state only what is left out and whatwingtip_distancedoes.
claude, rubric CLEAN_CODE.md. A different lab from the implementer
on purpose: a reviewer sharing its blind spots would not flag its mistakes.
| span = zeros(length(quarter_chord)) | ||
| for i in 2:length(span) | ||
| step = quarter_chord[i] .- quarter_chord[i-1] | ||
| span[i] = span[i-1] + hypot(step[2], step[3]) |
There was a problem hiding this comment.
MINOR: The card says this matches what refine_mesh_for_linear_cosine_distribution! measures, but that function (and compute_refined_section_interpolation!) uses full 3D quarter-chord length with x included. The repo now has two different ideas of span position, and refinement still places panels by the length this PR stops using. Either correct the card and say so, or explain why the two may differ.
|
Local full suite: PASS (6 min, Julia 1.13.0, one cell of the matrix) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| - The Makie `plot!` methods for a `Panel` or a `BodyAerodynamics` return a | ||
| `Vector{Makie.AbstractPlot}` instead of a `Vector{Any}`; for a `BodyAerodynamics` | ||
| drawn as flat panels it is one flat list rather than a list per panel. | ||
| - `obj_to_yaml` and `perpendicular_sections` spread the sections evenly over the span, |
The same mesh and wingtip_distance now give different section positions, and march_edges no longer returns arclen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The polar tables are written with four decimals, and a 1 deg deflection moves Cd by about 2e-5 on these sections, so the two curves are bit-identical once the fixture re-slices. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The cd check could not tell the two operating points apart, so it held whatever delta the plot used. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The cumulative-length helper allocated three arrays per call, which took refine! past its allocation budgets in test/bench.jl (UNCHANGED 12 > 5). spanwise_step now gives the length between two sections as a scalar and span_position walks the sections, so refine! allocates less than before the branch (UNCHANGED 0). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reverts 09d4e84 and 40f69c3. Only the slicer drops the chordwise step: there the leading edge turns into the side of the wing at no definite point. Refinement works on sections already cut and keeps measuring the bound vortex line, the local span direction of Gaunaa et al. 2026 (CP1). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TL;DR
station_indicesnow places then_sectionstargets evenly along the quarter-chord line with its chordwise (x) step dropped, instead of along the leading edge's arc length. At a raked or closing tip the leading edge runs almost straight aft, so arc-length targets piled into the last few centimetres of span, andwingtip_distancegave almost no inset there.What was wrong
march_edgessummed the full 3D leading-edge step intoarclen, andstation_indicesspread its targets over that. A station where the leading edge moves 40 mm aft for 3 mm of span counts as 40 mm, so the tip cap takes a share of the sections far beyond its span.The SK100 mesh from the issue isn't in the repo, so I reproduced the problem on a synthetic march: a straight leading edge over |y| ≤ 1 m, then 20 cap stations per side running 40 mm aft per 3 mm of span (half-span 1.06 m). With 9 sections, main gives:
A 0.3 m
wingtip_distancemoved the outermost section 2 cm. That is the "1.2 m of tip inset gives 4.9 cm of span" from the issue.What changed
station_indicescomputes the quarter-chord point of every marched station and adds uphypot(Δy, Δz)between neighbours. The targets, themin_chord_fractrim andwingtip_distanceall use that length now.march_edgesno longer returnsarclen.station_indiceswas its only reader.docs/src/airfoil_pipeline.md,docs/src/settings.mdand theWingSettingsfield doc now describewingtip_distanceas a spanwise length [m]. The changelog entry is under Changed, markedBREAKING:: the same mesh and the samewingtip_distancegive different section positions, so tuned values and the geometry generated from them have to be redone.test/plotting/test_plotting.jlnow askscl, notcd, whether the deflection reached the plot. That assertion is the red check this branch carried: the polar tables are written with%.4fandCd ≈ 0.009, so a 1° deflection movesCdby about2e-5and rounds away, leaving the deflected and undeflected curves bit-identical. On main it passed on one unit in the last written decimal — one bracketing section rounding to0.0099against the other's0.0098— which re-slicing the fixture takes away.clmoves by 0.038 over the same deflection, and the undeflected curve is pinned oncltoo, so that call is now checked against the operating point it is supposed to fall back to. The table precision itself is POLAR_MATRICES tables are written with %.4f, so Cd carries no deflection dependence at small deltas #362.Why not plain
y, the issue's first suggestion? The repo's ram-air kite curls down at the tips. Near station 80 the leading edge movesdy=0.0185, dz=-0.1206per step. Placing sections byywould spread the outer ones far apart along the real span. Dropping onlyxhandles both a curved span and a raked tip.Why drop
xonly here? At a closing tip the leading edge rakes aft until it meets the trailing edge, and somewhere along that curve it stops being a leading edge and becomes the side of the wing, at no point one could mark. A quarter-chord length that keepsxstill counts that side as span, so only dropping the chordwise step stops the cap soaking up sections: it spends 25.9 m of arc per metre of span.Refinement keeps the full 3D quarter-chord length.
LINEAR/COSINErefinement works on sections that are already cut, where the quarter-chord line is the bound vortex. That line is what Gaunaa et al. 2026 (TORQUE, CP1) define as the local span direction, and the solver's forces and perpendicular velocity are built on it. So the two measures differ on purpose: droppingxis a slicing rule for an ill-defined tip, not a new definition of span. A swept wing refined withLINEARtherefore gets panels of equal bound-vortex length, not equaly–zwidth. For a short while this branch moved refinement onto the slicer's measure too; f06df0b reverts that, and the tree is identical tobfb6487.On a strongly raked tip
A synthetic thick wing, chord 1 m, straight to |y| = 1 m, then an elliptic tip whose leading edge sweeps the full metre aft to meet the trailing edge over the last 10 cm of span. It is sliced into 12 sections with
wingtip_distance = 0.05and 240 march bins. All three columns share one march and one set of axes. The left column runs main'sobj_slice.jl, loaded fromorigin/main. The middle column is a probe that measures the quarter-chord length withxkept. The right column is this branch.xkeptxdroppedMain puts three of its 12 sections in the last 4 cm, outboard of the 5 cm inset it was asked for. Keeping
xin the quarter-chord length only halves the problem, because the side of the wing still counts as span. Droppingxputs the outermost section on the inset line and spaces the rest evenly.Effect on the repo's ram-air kite
The "before" panel ran main's
obj_slice.jl(loaded fromorigin/main) on the same march, with the samen_sections=10,wingtip_distance=0and axes. The largest panel span divided by the smallest drops from 1.44 to 1.10. This mesh has no long raked cap, so the change is modest here.With
wingtip_distance=0.05it is not modest, and not in the way I'd have liked. 5 cm of span inboard of the last sliceable station is marched station 79, where arc-length placement landed on station 80 and walked past it — and station 79 is a bad cut: it reports a section 0.407 m deep on a 0.506 m chord (t/c = 0.80) where stations 78 and 80 give 0.19 and 0.17. That is #361, a stale tangent inmarch_edgesthat predates this branch; I measured a four-line fix and left it there rather than in this diff, because it moves every station's frame on every mesh. Until it lands, this change makes that station reachable: atn_sections=4(the plotting fixture) the blob survives into the output and NeuralFoil clamps its whole table atCd = 1.0000.obj_to_yaml's own degeneracy guard catches it from aboutn_sections=10upward —thickness=0.428against a median of0.122trips2 × medianand the section borrows its neighbour's airfoil — so the exposure is meshes sliced into very few sections, where half the sections are tip blobs and the median that guard trusts is itself polluted.Where I'd push back
n_sectionsgets close to the number of stations, rounding dominates. On this mesh,n_sections=45with the defaultn_bins=60gets worse: the span ratio goes from 2.07 to 4.45, because the curled tip stations are about 0.12 m apart iny-zwhile the inboard ones are 0.055 m apart. Withn_bins=240it improves instead, from 1.90 to 1.36. Placing targets finer than the station spacing would mean interpolating between stations, which is a separate change.d_arc=0.0816, dx=0.0807, dy=0.0032) leave about 12 mm ofzper station. If thatzis the nose sliding vertically as the section closes, rather than real span, those stations still count about 3.8× theirystep (it was 25.9×). Someone with that mesh should rerun the panel-sweep table.wingtip_distancevalues and generated geometries already out there were placed the old way.obj_to_yamlreuses an existinggeometry.yaml, andtest/generated/is keyed on settings rather than on the code, so both have to be regenerated.Left out
min_chord_fracthrough toWingSettings. The issue calls it independent, and it would add a new setting. With a spanwisewingtip_distanceit may no longer be needed; I'd open it separately if it still is.z_airfconditioning on near-streamwise panels. That is Panel normal z_airf is built from the leading-edge step, not the quarter-chord step the bound vortex uses #272 / Take the panel normal from the quarter-chord step #273.Verification
FieldError: type NamedTuple has no field arclen; witharclensupplied it fails on the numbers.test/obj_adapter/test_obj_adapter.jl"station_indices spreads sections evenly in span past a raked tip": green after the change (3/3, juliaserver)test/generated/cleared —POLAR_MATRICES panel is evaluated at the passed deltafailed ondeflected[2] != stored[2]with CI's own numbers to 15 digits (0.6330187742107254, …), 111 passed / 1 failed; green after both test commits, 112/112 (juliaserver, Julia 1.13)test/generated/rebuilt by this code:obj_adapter/test_obj_adapter.jl55/55 ·settings/test_settings.jl33/33 ·test_refinement_validation.jl15/15 ·wake/test_wake.jl3/3 ·body_aerodynamics/test_results.jl30/30 ·solver/test_forwarddiff.jl7/7 (on9af0690); after mergingmain'sSolverconstructor deprecation, the two files that merge touched re-run green:test_obj_adapter.jl55/55 andtest_plotting.jl112/112 (juliaserver, Julia 1.13.0)f06df0b) · no REUSE lint in this repo · merges cleanly with currentmainPkg.test()on Julia 1.13.0 withtest/generated/regenerated: PASS onf06df0b, 6713 passed and 1 broken (the existing@test_skipintest_kite_geometry.jl), 5m · GitHub CI: PASS onf06df0b, Julia 1.12 and 1.13 on Linux, macOS and Windows, plus docs and the setup workflowwingtip_distance = 0.05, 12 sections, gap ratio 38.7 on main and 1.1 on this branch (script in the section above, main's slicer loaded fromorigin/main)wingtip_distancenow walks a mesh into stations arc length used to skip, and one of them on this repo's own kite is the bad cut of march_edges gives each station the previous station's leading-edge step, so a cut near a closing tip grazes the surface and reports an 80%-thick section #361.Scope
+62 / −35 across 8 files.
obj_slice.jlis about net zero:arclencomes out ofmarch_edgesand the spanwise sum goes intostation_indices. Also in the diff: the new test (+19), the changelog (+7), docstring and doc wording, and two lines oftest_plotting.jl. Rewrapping theobj_to_yamldocstring paragraph is the only tidy-up. I searched forarclen,station_indices,wingtip_distance, "arc length" and quarter-chord helpers; none existed to reuse.Closes #275 · task
VortexStepMethod.jl-275