Truncate the SVD in the least-squares Kulfan fit, and warn when it does, so crowded stations give bounded weights

[1-Bort-1]

TL;DR

fit_kulfan_parameters(x, y, LeastSquaresFit()) now solves with a truncated SVD instead of A \ y, dropping singular values below 1e-4 of the largest and warning when it drops any. A contour whose stations crowd into a narrow band of the chord made the solve return weights that kulfan_to_coordinates blew up to 1e4 scale, and nothing downstream checks magnitude.

What was wrong

When most stations on one surface sit inside a narrow band of x, the upper Bernstein columns become numerically dependent (cond 1e12–1e15). The QR solve then fits the non-representable part of the contour with huge, cancelling weights, which only cancel on the stations they were fitted against. I reproduced this without the SK100 mesh: 118 upper stations in [0.30, 0.31] on a known CST shape with a 0.005 ripple on top gives a resampled max|y| of 13946 against a true 0.077. That is the same size as the 13013 in the issue.

Why truncation and not the KulfanBasis ridge

The obvious fix was the Tikhonov ridge KulfanBasis uses. It does not work here because "Fit matches aerosandbox get_kulfan_parameters" pins the fitted weights to AeroSandbox's at 1e-9. A ridge λ changes each weight by about λ·cond². At a normal cond of about 1e3, that means either λ ≤ 1e-16, which does nothing for a crowded fit, or every fit drifts off AeroSandbox. A hard SVD cutoff returns exactly the same answer whenever cond(A) < 1e4, and the minimum-norm answer when it is higher.

Measured by the smallest kept singular value relative to the largest (cond of A):

contour	cond	\ max |y| resampled	truncated SVD, rtol 1e-4	change in weights vs \
test_airfoil.dat	916	0.0964	0.0964	6.8e-15
spread stations, ripple	1120	0.0768	0.0768	6.7e-15
118 stations in 0.01 band, ripple	1.2e15	13900	0.119	—
118 stations in 0.03 band, ripple	1.5e12	629000	1.12	—

The cutoff allows a cond of 1e4, about 9× the worst normal cond I found (1120). At 1e-3 it would already start cutting test_airfoil.dat, whose cond is 916.

The warning

truncated_least_squares(A, b) does the SVD once and returns the weights together with the number of singular values it dropped. fit_kulfan_parameters warns once per fit when that number is not zero: on the crowded test contour it says Kulfan fit dropped 6 of 18 singular values: the stations leave part of the shape unconstrained, as when they crowd into a narrow band of the chord. The count is taken from the final solve, so a trailing-edge refit does not warn twice. None of the sections built by test_live_polar.jl or test_backend_comparison.jl triggers it.

Where I would push back

• Truncation bounds the weights, but it cannot recover shape the stations never sampled. The 0.03-band row still comes out 15× too thick, only no longer 1e4×. The real cause is the resampler: smoothed_curvature has no clamp on its density, and resample_arc crowds the stations. That fix, and the thickness check in write_section_aero, are options 2 and 3 of the issue. Neither is in this branch. With both, a degenerate section would error instead of being written.
• KulfanBasis keeps its ridge. Its job is to bound a deflection projection, not to match a reference fit, so the two remedies do different things. I searched src/ for ridge|svdvals|pinv|Tikhonov and found no third regularised solve.
• Cleanup around the change: the trailing-edge refit reuses A[:, 1:end-1] instead of rebuilding the hcat. The result is the same.

Verification

• Reproduced first: the new testset on unchanged src/, Evaluated: 13946.420825128731 < 0.1631808615770601
• test/airfoil_aero/test_airfoil_aero.jl "Fit to stations crowded into a narrow band stays airfoil-sized": red before, green after (juliaserver). The warning's @test_logs fails on the pre-warning src/ with Captured Logs: empty and passes after; the same testset asserts that a fit of well-spread stations logs nothing.
• Full test/airfoil_aero/test_airfoil_aero.jl passes, including the AeroSandbox reference and round-trip testsets. test/airfoil_aero/test_live_polar.jl and test/solver/test_backend_comparison.jl pass too (juliaserver, 0 fail / 0 error)
• Merged main (1a80247). The only conflict was the CHANGELOG.md Fixed list, where both sides' entries are kept. After the merge, test_airfoil_aero.jl and test_live_polar.jl pass on Julia 1.13 (juliaserver, 0 fail / 0 error).
• After the warning commit b725371: test_airfoil_aero.jl, test_live_polar.jl and test/solver/test_backend_comparison.jl pass (juliaserver, Julia 1.13, 0 fail / 0 error). No warning was logged.
• Local full suite (agent ci-local, Julia 1.13) on b725371: PASS (7 min)
• Docs build: not run locally, because the docs environment has no manifest on this box. The CI Documentation job on b725371 passes.
• GitHub CI on b725371: PASS in every job (Documentation, setup, windows/macOS/ubuntu 1.12, ubuntu 1.13). Before the warning commit, windows 1.12 failed on the merged head at test/solver/test_forwarddiff.jl:95 (POLAR_MATRICES, 0.046430340012943265). That is test_forwarddiff.jl's POLAR_MATRICES check flakes at ~4%, and the oper #287: main's own push run on 086ee32 fails with the same bits (run 35334134759).
• REUSE: not used in this repo. Up to date with main.
• Benchmark: n/a
• Risk: a real contour with cond between 1e3 and 1e4 would now fit slightly differently from AeroSandbox. Of the contours I checked, none came near that range.

Scope

+44 / −4 across 4 files. In src/airfoil_aero/kulfan.jl: a constant, the 13-line truncated_least_squares helper, and a three-line warning in the fit. Also one testset with the warning's two @test_logs, the changelog entry, and the helper's line on the private-functions docs page.

Closes #295 · task VortexStepMethod.jl-295

[1-Bort-1]

Independent review (advisory)

Verdict: APPROVE WITH COMMENTS · 2 inline, 0 off the diff

Good

• The diff matches the card: git diff --stat HEAD~1 shows +22/−4 across CHANGELOG, kulfan.jl and one testset, with nothing unplanned
• Switching A \ y_norm to pinv(A; rtol) changes no result where cond(A) < 1e4, since both return the least-squares solution; the AeroSandbox 1e-9 testset still covers this path
• The refit A[:, 1:end-1] is the same matrix as the old hcat without te_col (the last column), so the named cleanup is behaviour-preserving and saves a second allocation
• LinearAlgebra is already loaded in AirfoilAero.jl, and the test file already imports class_function and bernstein_basis, so no new imports are needed
• No AD path goes through the fit: grepping src/airfoil_aero finds no ForwardDiff or Dual, so the SVD in pinv cannot break a Dual-number caller
• Every caller (geometry_gen, live_polar, section_aero_gen, common.jl) runs at build or generation time, not in the solve loop, so the SVD's extra cost does not matter
• The card explains why a ridge was rejected (it would break the AeroSandbox pin), and a grep for a third regularised solve found none, so there is no duplicate path
• The new testset is named for the behaviour it protects, and the card shows it failing before the fix (13946 < 0.163)

Not good

• CHANGELOG.md:18 — The entry says a crowded contour now fits to an airfoil-sized shape, but the card's own table shows the 0.03-band case still comes out 15× too thick. The entry should say the weights are bounded, not that the shape is correct, or users will trust sections that are still wrong.
• src/airfoil_aero/kulfan.jl:150 — Truncation happens silently, so a degenerate contour now gives a plausible-looking but wrong section instead of an obviously broken one, and nothing downstream checks it. Without the resampler or thickness fixes in this branch or a tracked issue for them, the failure gets harder to spot.
• The cond survey covers test_airfoil.dat plus synthetic contours, but the real inputs are shrink-wrapped kite sections (deform_section, panel_kulfan_parameters); the TUDELFT_V3 and ram-air sections should be checked against the 1e4 cutoff
• A hard cutoff is discontinuous: a section whose cond crosses 1e4 during a deform_section delta sweep would jump in weights between neighbouring deflections, and nothing warns
• Closes #295 shuts the issue while options 2 and 3 (clamping resampler density, a thickness check in write_section_aero) stay undone and are not linked to a follow-up issue
• The docstring and CHANGELOG repeat the literal 1e-4 instead of naming KULFAN_FIT_RTOL, so they go stale if the constant changes
• The testset defines a local surface(weights, xs) closure with logic in it; §6 prefers a hoisted helper, though this one is a one-liner
• The local CI mirror result was not read and GitHub CI is still pending, so the only green runs are on juliaserver

_{claude, 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]

MINOR: The entry says a crowded contour now fits to an airfoil-sized shape, but the card's own table shows the 0.03-band case still comes out 15× too thick. The entry should say the weights are bounded, not that the shape is correct, or users will trust sections that are still wrong.

[1-Bort-1]

Fixed in b725371: the entry now says the weights are bounded and that the fit warns. It no longer claims an airfoil-sized shape.

[1-Bort-1]

MINOR: Truncation happens silently, so a degenerate contour now gives a plausible-looking but wrong section instead of an obviously broken one, and nothing downstream checks it. Without the resampler or thickness fixes in this branch or a tracked issue for them, the failure gets harder to spot.

[1-Bort-1]

Fixed in b725371: the fit now warns once, with the count of dropped singular values, whenever it truncates.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[1-Bort-1]

Local full suite: PASS (4 min, Julia 1.13.0, one cell of the matrix)

[1-Bart-1]

Add the warning

[1-Bort-1]

Added in b725371: the fit now warns once whenever it drops singular values, with a test that the crowded contour warns and a well-spread one does not.