Skip to content

Spread sliced sections evenly over the span, not over leading-edge arc length - #350

Open
1-Bort-1 wants to merge 1 commit into
mainfrom
agent/275-section-slicing-spreads-stations-by-lead
Open

1-Bort-1 wants to merge 1 commit into
mainfrom
agent/275-section-slicing-spreads-stations-by-lead

Conversation

@1-Bort-1

Copy link
Copy Markdown
Contributor

TL;DR

station_indices now places the n_sections targets 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, and wingtip_distance gave almost no inset there.

What was wrong

march_edges summed the full 3D leading-edge step into arclen, and station_indices spread 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:

y    = [-1.06, -1.027, -0.9, -0.45, 0.0, 0.45, 0.9, 1.027, 1.06]
diff = [0.033, 0.127, 0.45, 0.45, 0.45, 0.45, 0.127, 0.033]   (even spacing: 0.265)
wingtip_distance = 0.3 → outermost y = ±1.039                 (a 0.3 m inset: ±0.76)

A 0.3 m wingtip_distance moved 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_indices computes the quarter-chord point of every marched station and adds up hypot(Δy, Δz) between neighbours. The targets, the min_chord_frac trim and wingtip_distance all use that length now.
  • march_edges no longer returns arclen. station_indices was its only reader.
  • Docstrings, docs/src/airfoil_pipeline.md, docs/src/settings.md and the WingSettings field doc now describe wingtip_distance as a spanwise length [m]. The changelog entry is under Changed.

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 moves dy=0.0185, dz=-0.1206 per step. Placing sections by y would spread the outer ones far apart along the real span. Dropping only x handles both a curved span and a raked tip.

Why the quarter chord? It is the line the bound vortex lies on, and it is what refine_mesh_for_linear_cosine_distribution! already measures. Dropping x removes the part of each step that points along the freestream, which is what made those panels degenerate.

Effect on the repo's ram-air kite

Ram-air kite front view, 10 sections: before, placed by leading-edge arc length; after, evenly over the span

The "before" panel ran main's obj_slice.jl (loaded from origin/main) on the same march, with the same n_sections=10, wingtip_distance=0 and 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. The tips that close to a point are still trimmed as before (tip chord 0.168 m in both).

Where I'd push back

  • Sections still snap to the nearest marched station. When n_sections gets close to the number of stations, rounding dominates. On this mesh, n_sections=45 with the default n_bins=60 gets worse: the span ratio goes from 2.07 to 4.45, because the curled tip stations are about 0.12 m apart in y-z while the inboard ones are 0.055 m apart. With n_bins=240 it 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.
  • I couldn't check the SK100 cap. Its numbers in the issue (d_arc=0.0816, dx=0.0807, dy=0.0032) leave about 12 mm of z per station. If that z is the nose sliding vertically as the section closes, rather than real span, those stations still count about 3.8× their y step (it was 25.9×). Someone with that mesh should rerun the panel-sweep table.
  • Old outputs don't regenerate on their own. The wingtip_distance values and generated geometries already out there were placed the old way. obj_to_yaml reuses an existing geometry.yaml, and test/generated/ is keyed on settings rather than on the code, so both have to be regenerated.

Left out

Verification

  • Reproduced first on a synthetic raked-tip march: main gives the spacing and inset shown above. On main the new test's march errors with FieldError: type NamedTuple has no field arclen; with arclen supplied 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)
  • Affected files, with test/generated/ rebuilt by this code: obj_adapter/test_obj_adapter.jl 55/55 · settings/test_settings.jl 33/33 · test_refinement_validation.jl 15/15 · wake/test_wake.jl 3/3 · body_aerodynamics/test_results.jl 30/30 · solver/test_forwarddiff.jl 7/7
  • plotting/test_plotting.jl (calls station_indices through plot_slices_3d): 58/58 in the local CI mirror
  • Docs build clean (docs/make.jl) · up to date with main · no REUSE lint in this repo
  • Local CI mirror, full Pkg.test() on Julia 1.12.7: PASS, 6421 pass and 1 broken (the existing @test_skip in test_kite_geometry.jl), 14 min · GitHub CI: runs when this PR opens
  • Benchmark: n/a
  • Risk: generated geometries change section positions. No test pins those numbers, but downstream users with a tuned wingtip_distance get a different inset.

Scope

+56 / −33 across 7 files. obj_slice.jl is about net zero: arclen comes out of march_edges and the spanwise sum goes into station_indices. Also in the diff: the new test (+19), the changelog (+4), and docstring and doc wording. Rewrapping the obj_to_yaml docstring paragraph is the only tidy-up. I searched for arclen, station_indices, wingtip_distance, "arc length" and quarter-chord helpers; none existed to reuse.

Closes #275 · task VortexStepMethod.jl-275

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

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

Copy link
Copy Markdown
Contributor 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 · 1 inline, 0 off the diff

Good

  • The fix matches the card: station_indices now spreads targets over the quarter-chord hypot(Δy, Δz) length, and the min_chord_frac trim and wingtip_distance use that same length (obj_slice.jl:464-473).
  • Removing arclen from march_edges is safe: a repo-wide rg arclen finds no other reader, and plot_slices_3d in the Makie extension only calls station_indices.
  • The docs are consistent: settings.jl, settings.md, airfoil_pipeline.md, both docstrings and the changelog all call wingtip_distance a spanwise length [m], and rg '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_yaml docstring, as the card says, and min_chord_frac passthrough and #272/#273 are left out on purpose.

Not good

  • src/obj_adapter/obj_slice.jl:468 — 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.
  • The test has z = 0 everywhere, so it only checks that x is dropped. Changing the code to use plain y would still pass, even though the card's reason for including z is the curled ram-air tip.
  • If closed tip stations step only in x, their span values equal span[first(usable)]. argmin then returns the first of the tied indices, which is a closed station, and that silently undoes the min_chord_frac trim on the left tip. This is unlikely on real meshes but worth a line in the card.
  • The station_indices docstring 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 what wingtip_distance does.

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])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

@1-Bort-1

Copy link
Copy Markdown
Contributor Author

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

@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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

Section slicing spreads stations by leading-edge arc length, which collapses to nothing in span at a raked tip

1 participant