Fit the contour generate_airfoils is handed, not a second wrap of it

[1-Bort-1]

TL;DR

generate_airfoils shrink-wrapped a contour both of its callers had already wrapped, and that second wrap tips the least-squares Kulfan fit into rank deficiency for about a fifth of the SK100's sections, writing their airfoil .dat files at 1e4 scale. It now fits the contour it is given — which is what its own docstring says it receives — so the wrap_method the caller configured is the only one that runs. Closes #293.

What was wrong

Both front ends wrap before they call. obj_to_yaml does it at obj_to_yaml.jl:221 and the Surfplan adapter at SurfplanAdapter.jl:71, each with the wrap_method its settings name, and both hand the result in as x_fit/y_fit. generate_airfoils then called generate_airfoil_aero(solver, x::Vector, y::Vector), whose whole job is to shrink_wrap a raw slice before fitting — and with a bare ShrinkWrap(), so the configured wrap was quietly replaced by the default one as well.

Wrapping a wrapped contour is not a small perturbation of the fit. The second wrap rounds an already-rounded convex corner into an arc of near-coincident vertices; smoothed_curvature divides the turn across a span of order 1e-16, so resample_arc's curvature-weighted measure blows up and crowds most of the upper stations into one narrow band of x. The upper Bernstein block of the fit matrix then goes numerically singular — on SK100 section 16, cond(A) moves from 1.07e3 to 2.52e11 — and A \ y_norm at kulfan.jl:147 returns weights of order 1e5. The residual is normal, because those weights cancel on the stations they were fitted to; kulfan_to_coordinates re-evaluates them on its own cosine grid, where they do not, and y reaches -13013.

Nine of the 45 sections come out that way, and the contours written from them — airfoils/{6,8,9,10,12,16,27,29,43}.dat — carry coordinates from 1e2 to 1e4 where a clean section is 0..1. Nothing in the pipeline notices: the polars still solve, the node counts are still right, the run reports every airfoil as converged, and the geometry is only refused much later by a consumer that maps the mesh onto a structure. In SymbolicAWEModels that reads AeroPressure wing 1: max surface-node→point distance is 35776.8× the local chord, exceeding frame_tol_frac=2.0, which is where this was found: it is what stopped 1-Bart-1/BeyondTheSim.jl#17 from building a model at all.

How the test pins it

The written contour has to be the fit of the contour that was handed in. Wrapping again inflates the section by the default clearance, 0.006, so the test measures the extremes of the written .dat against kulfan_to_coordinates(fit_kulfan_parameters(x_fit, y_fit, LeastSquaresFit())) and allows 1e-4. With the second wrap that gap is 6.27e-3; without it, 6.31e-6 — the resampling error, three orders of magnitude clear of the tolerance. It uses the suite's own test_airfoil.dat, so it needs no fixture, and it fails on any airfoil rather than only on the sections whose fit happens to blow up.

Where I would push back

This removes the trigger, not the fragility. The fit at kulfan.jl:147 is an unregularised A \ y with no rank check, no coefficient bound and no check on what kulfan_to_coordinates then produces — while KulfanBasis in deform.jl:44 already ridge-regularises the same kind of solve. Any other input that crowds the resample will do the same thing just as silently, and deform_section still wraps once more per deflection on a default ShrinkWrap() rather than the caller's. Raised separately rather than widened into this diff.

One thing worth knowing when judging generated tables: a blown-up section is not always obvious from its bounding box. 10_d10.dat looks normalised because it was rescaled by a large chord — its lower surface is a constant min_clearance/chord. Thickness is the honest check, not x ∈ [0, 1].

Verification

• Reproduced first: airfoils/10.dat starting 144.26245312 47009.20858195, and the consumer's max surface-node→point distance is 35776.8× the local chord
• test/airfoil_aero/test_airfoil_aero.jl red before the fix (1 passed, 1 failed), green after (all testsets pass, exit 0)
• test/obj_adapter/test_obj_adapter.jl not run locally: this worktree has no juliaserver session of its own — its directory name collides with another task's upstream worktree — so the tests were run from the dependent's environment, which does not carry the suite's YAML and Aqua. GitHub CI is the authority for the rest of the suite.
• Risk: the fix changes generated geometry for every user of both adapters. Tables generated before it do not reproduce after it — a section that was fitting a doubly-wrapped contour now fits the caller's, so its polars move by the difference between one wrap and two.

Scope

+19 / -1 across 3 files: the one call in src/airfoil_aero/geometry_gen.jl, its regression test, and the changelog entry. No stack — no open pull request here touches either file.

From 1-Bart-1/BeyondTheSim.jl#17 · task BeyondTheSim.jl-17

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[1-Bort-1]

This is the case that made me want figures in PR cards, so let's do it here first.

The argument is entirely visual and the diff is three lines — a reader can't check cond(A) 1.07e3 → 2.52e11 by reading it, but they can see that a blown-up section stops looking like an airfoil. Add to the card:

• 2D slice, before vs after, for at least one of the sections that break ({6,8,9,10,12,16,27,29,43}), on identical axes, against the contour that was handed in. Plot thickness rather than the bounding box — you noted yourself that 10_d10.dat looks normalised because a large chord rescaled it, and a bounding-box figure would hide exactly that.
• Section polars for the same sections, before and after, over several delta. The generated polars moving is the real risk of this change, so show it.

Render the before from the merge base in a separate worktree, same inputs and same axes limits as the after. Existing exports in the Makie ext (plot_airfoil_fit, plot_airfoils, plot_section_polars) should get you most of the way; new functions there are fine if they're worth shipping to users.

Hosting is 1-Bart-1/Agents#226 and may not be ready. If it isn't, say so here and hold — don't commit PNGs to this branch. Keep the existing text; the figures are evidence for it, not a replacement.

[1-Bort-1]

The fix is right and the test pins it at δ=0. What it doesn't cover is the δ grid.

generate_airfoils writes airfoils/{id}_d<δ>.dat and the _cp/_cf node tables off generate_airfoil_aero's per-δ columns, and those inherit the base fit: kulfan_to_coordinates(base) → deform_section per δ (rotate about crease_frac, re-wrap at clearance=0, refit). The new test leaves delta_range unset, so only the δ=0 contour is checked — while on the SK100 the deflected files were wrong too, 10_d10.dat with its lower surface pinned at a constant min_clearance/chord.

Worth adding, here or as a follow-up: one generate_airfoils run with a delta_range spanning both signs over a wrapped contour — the suite's own test_airfoil.dat, wrapped, as this test already does — asserting every written _d<δ>.dat on thickness and chordwise extent. Thickness rather than bounding box: a section rescaled by a large chord passes extrema(x) ≈ [0, 1], which is the form the existing deform_section flap assertions use.

The unregularised fit underneath is #295.

[1-Bort-1]

No figures yet, and none committed: there is nowhere to put them.

1-Bart-1/Agents#226 (orphan assets branch, publish helper, the AGENTS.md §6 rule) is open and still agent:needs-plan-approval. Neither this repo (78 branches) nor Agents has an assets branch, and the agent CLI has no publish command — so a Markdown image has no URL to point at, and AGENTS.md §9 rules out committing the PNGs to this branch instead. #297 holds until #226 lands; this PR is untouched.

Two things worth knowing before anyone renders the pair, both in full on #297:

The subject is private, the host would be public. The sections to show are SK100, and that .obj is in the private 1-Bart-1/BeyondTheSim.jl, while the assets branch would be on this public repo with each URL pinned to a SHA and never rewritten. Rendering "the real case" therefore publishes kite geometry to a permanent public URL. That is a call to make before the first figure, not after.

Neither figure is producible with the exports as they stand. plot_airfoils(geometry_file; idxs=…) draws the right content — fitted line over the _raw.dat points — but has no limits keyword and autoscales each subplot under aspect=DataAspect(), so a section at 1e4 and a clean one land on different axes; #226's own rule says a pair differing in anything but the fix is not evidence. plot_section_polars filters on POLAR_VECTORS and throws on the POLAR_MATRICES grid that a delta_range run writes, plots every section with no selector, exposes no limits, and returns a PlotX rather than a figure. Nothing exported plots coefficients against α at several δ in 2-D: plot_polar_data is the only thing that reads the (α, δ) matrix and it is a 3-D wireframe of panels[1]. So the figures need new exported API here — one PR's worth, and its own decision.

[1-Bart-1]

@1-Bort-1 Add a delta range test to this PR

[1-Bort-1]

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

[1-Bort-1]

Blocked. 3 runs in the last hour ended in an error, so this one is not being started again:

Error: cannot check out agent/BeyondTheSim.jl-17 for VortexStepMethod.jl-294: fatal: 'agent/BeyondTheSim.jl-17' is already used by worktree at '/home/agent/worktrees/BeyondTheSim.jl-17/.agent/upstream/OpenSourceAWE__VortexStepMethod.jl'

[1-Bart-1]

Fix the error, try again