Re-wrap a deformed section with the ball it was first wrapped with, and cut the loops out of the clearance offset

[1-Bort-1]

TL;DR

deform_section now re-wraps with the rolling ball the section was first wrapped with, at zero clearance, and shrink_wrap rolls that ball on the polygon of a closed input instead of only on its nodes. A thin wrapped canopy section used to come back from deform_section with its two skins crossing, at every flap deflection. The clearance offset of the first wrap also crossed itself on thin sections, 6 of 18 V3 sections at the default MeshSettings, and it now has its loops cut out. Both wraps now come back as simple closed curves on all 18 V3 sections at every radius, clearance and deflection tested, and the default settings no longer warn.

What was wrong

deform_section re-wrapped every deflection with a fixed ShrinkWrap(clearance=0.0), a ball of radius 0.02, whatever the section was first wrapped with. A single-skin canopy wrapped once is a capsule about 0.002 thick, with panels up to 0.09 long in mid-chord. shrink_wrap treats its input as loose points, so between two nodes a distance h apart the ball dips h²/8r below the panel. Past h ≈ 0.018 at r = 0.02 that dip is more than the capsule is thick, and the ball touches the other skin. The traced loop then switches between skins 18–74 times, and resampling it turns the pinches into crossing panels.

The flap rotation is not the cause: rotating without re-wrapping gave 0 crossings, and the same sections crossed at δ = 0, 0.01°, 5° and 15° alike. Handing the re-wrap the first wrap's radius, as asked on the thread, is not enough on its own. At r = 0.2, 6 of 18 V3 sections still crossed, because the first wrap's panels there are 0.08–0.1 long and even a ball of 0.2 bulges 0.0044 past such a panel, twice the thickness.

What changed

• deform_section(x, y, delta; wrap_method) takes the ShrinkWrap the section came from. The re-wrap uses its radius, min_clearance, n_points and curvature_weight with clearance = 0, since re-applying the first pass's clearance thickened the section a second time (area 0.0190 → 0.0278 on one V3 section). The same wrap_method keyword is threaded through generate_airfoils, generate_airfoil_aero, generate_aero_matrices and generate_polar_from_coordinates. obj_to_yaml, the Surfplan adapter and the two Makie previews pass the one they wrapped with. The default is ShrinkWrap(), which re-wraps with exactly the ball the old default did.
• shrink_wrap splits the edges of a closed input to at most min(0.01, min_concave_radius/2) before rolling, so the ball rolls on the contour rather than falling between its nodes. Raw .obj slices are already densified to 0.005 chord by build_section, so their first wrap is unchanged. densify_contour moves from ObjAdapter into the shrink wrap for this, unchanged apart from keeping its input's point type.
• shrink_wrap warns, naming the ball radius and clearance, when the contour it returns crosses itself. It uses crossing_panels, about 0.07 ms on a 239-node contour. That check came from Refuse a contour XFoil's panel code cannot take #320, which closed unmerged; this branch keeps crossing_panels and its two helpers and leaves out Refuse a contour XFoil's panel code cannot take #320's XFoil guard.
• pivot_contour cuts out the loops its clearance offset makes (cut_loops), which fixes A wrap at the default clearance crosses itself on thin canopy sections #363 here as asked on the thread. On a thin canopy, where contacts on the two skins lie closer than clearance, the offset polyline turns back on itself: 4–19 loops on each of the three sections I counted (2, 9 and 16), most spanning two or three nodes. cut_loops walks the offset from its first node, and where a new edge crosses an earlier one it drops everything in between and keeps the crossing point. At the default clearance = 0.006, min_concave_radius = 0.02, 6 of 18 V3 slices crossed before and none after. At r ∈ {0.02, 0.05, 0.2} × clearance ∈ {0, 0.006, 0.02}, none of the 162 wraps cross, where 0.02 used to cross on all 18 at the two smaller radii. It is a quadratic walk over the offset nodes: on a 1931-node V3 offset it takes 6.3 ms, so a first wrap goes from about 2 to 8 ms. The zero-clearance re-wrap in deform_section goes from 0.68 to 0.91 ms.
• segments_cross now also requires the two segments' extents to overlap. Along a straight edge its side-of-line products are rounding noise of either sign, so two disjoint panels on one straight nose edge were reported as crossing. That was the first CI run's only real failure, on all three cells: the section came out slightly differently under CI's freshly resolved dependencies. Locally, a sweep of 6400 re-wraps (both meshes, 10–40 sections, r = 0.1–0.3, δ = 0 and 5°) had 15 such false crossings before the fix and none after.
• generate_section_aero(solver, x, y) now calls generate_airfoil_aero(solver, x, y) instead of repeating its wrap-and-fit.

Section 7 of V3_25.obj (perpendicular_sections(…, 18; n_bins=100)), wrapped with ShrinkWrap(clearance=0, min_concave_radius=0.2), then deform_section(…, 0.0; wrap_method=same). Before: crossing_panels = (55, 189). After: nothing.

Crossing sections out of 18 V3 sections, first wrap and deform_section at δ = 0, 5, −5, 15° (72 per row)

first wrap r	clearance	first wrap crosses	deformed crosses, before	deformed crosses, after
0.02	0	0	65/72	0/72
0.05	0	0	—	0/72
0.2	0	0	72/72 (default re-wrap) · 24/72 (same r)	0/72
0.4	0	0	—	0/72
0.02	0.006	6 → 0	—	0/72
0.05	0.006	2 → 0	—	0/72
0.2 / 0.4	0.006	0	—	0/72

The "before" figures come from the investigation scripts on the base branch. The "after" column is this branch. "6 → 0" is before and after cut_loops.

Where I would push back

• The re-wrap is not exactly idempotent. Even without crossings, re-wrapping a clean section at r = 0.2 moved its lower surface by up to 0.020 near x ≈ 0.27 (area 0.01876 → 0.02021 on section 10). That is the rolling ball bridging a concavity of the first wrap, and it is unchanged here.
• The V3 geometry moves where the offset looped. On the sections whose clearance offset crossed, the first wrap now follows the outer loop instead of the tangle, so their polars change. The data files in data/ are not regenerated here.
• This moves every generated polar a little. Densifying applies to every closed input with edges longer than 0.01. That includes kulfan_to_coordinates output (up to 0.016) in the obj_to_yaml path, so its deflected shapes shift slightly even though none of them crossed before. The CHANGELOG lists this under Changed.
• The negative-flap hinge is at the wrong height. get_lower_upper puts it near the chord line on cambered sections. That is get_lower_upper puts the negative-flap hinge near the chord line on cambered sections #364.

Verification

• Reproduced first: deform_section of V3 sections wrapped at r = 0.2 with the same wrap_method crossed on 12 of 36 section × δ pairs (sections 3, 4, 6, 7, 13, 14).
• test/airfoil_aero/test_airfoil_aero.jl "re-wrapping a wrapped section keeps it a simple closed curve": 46 of 75 failed on the base branch and all pass after (juliaserver). The 75 checks are now split across three testsets: the V3 sweep (72), the clearance-padded re-wrap (2), and "shrink_wrap warns when its contour crosses itself" (2). The last one uses a wavy membrane at clearance 0.05 that crosses, with the same curve at clearance 0 as a control that must not warn. It no longer depends on the A wrap at the default clearance crosses itself on thin canopy sections #363 defect. The whole file passes after the review fixes.
• "shrink_wrap cuts the loops its clearance offset makes" (18 V3 slices at ShrinkWrap(), no warning and no crossing, plus the wavy membrane at clearance 0.05 without a warning): 13 of 37 failed with pivot_contour returning the raw offset, and 37 of 37 pass with cut_loops (juliaserver). That membrane no longer crosses, so "shrink_wrap warns when its contour crosses itself" now uses a closed wavy loop 2·10⁻⁴ thick at clearance 0, whose resampled skins cross, with the same loop 2·10⁻³ thick as the control that must not warn. The whole of test_airfoil_aero.jl passes, and so do test_obj_adapter.jl (54/54), test_live_polar.jl and ram_geometry/test_kite_geometry.jl.
• Affected files green in juliaserver: airfoil_aero/test_airfoil_aero.jl, airfoil_aero/test_live_polar.jl, obj_adapter/test_obj_adapter.jl, surfplan/test_surfplan.jl, settings/test_settings.jl, solver/test_backend_comparison.jl, plotting/test_plotting.jl.
• crossing_panels testset: the collinear-edge case (real node coordinates from the V3 section) failed before the segments_cross fix and passes after. test_airfoil_aero.jl and test_obj_adapter.jl green after it.
• ci-local (Julia 1.13) and GitHub CI on every earlier head of this branch: PASS.
• Up to date with main (dcd4f10). Merging Polars follow table_format through the node-table I/O, and lose csv from their names #372's Arrow polar writers conflicted only in generate_polar_from_coordinates, resolved to main's write_polar/write_polar_matrix and table wording plus this branch's wrap_method; merging Each panel takes its own wing's spanwise direction in calc_forces! and calculate_results #367/Take the panel normal from the quarter-chord step #273 conflicted only in two changelog Fixed entries, both kept. After the merges: test_airfoil_aero.jl, test_obj_adapter.jl (63/63), test_surfplan.jl (8/8), test_load_polar_data.jl (27/27) green. ci-local (Julia 1.13) on dcd4f10: PASS in 6 min. GitHub CI on dcd4f10: every job passes (Julia 1.12 on ubuntu, macOS and Windows, Julia 1.13 on ubuntu, setup test, Documentation, codecov/patch).
• Docs: the CI Documentation job passes on ee7a6b3. Not built locally, because docs/ has no manifest. No new public symbol. densify_contour moves to the AirfoilAero private list, and cut_loops is added there.
• Risk: cut_loops keeps the piece that holds the offset's first node, at the leftmost cloud point. A loop that enclosed that node would be kept in place of the airfoil. No V3 or ram-air section does this.

Scope

+265 / −74 across 13 files, against main. The logic is cut_loops (+33), shrink_wrap (densify and warn, +15), deform_section (the re-wrap settings) and crossing_panels with its segment helpers (+45, from #320 plus the extent check). The rest is the wrap_method keyword threaded through the six callers and their docstrings, the densify_contour move (−21 in ObjAdapter, +19 in the shrink wrap), the tests (in four testsets) and the changelog. Based on main; #320's commits are in the history, but its XFoil guard is taken back out, so the diff against main holds only this change.

Closes #363

Closes #321 · task VortexStepMethod.jl-321

[1-Bort-1]

Independent review (advisory)

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

Good

• The fix matches the card: deform_section re-wraps with the caller's ball at zero clearance, and wrap_method is passed through all six callers named in the card (checked with grep in obj_to_yaml.jl:221/239 and SurfplanAdapter.jl:71/76).
• The new default ShrinkWrap() with clearance forced to 0 gives the same re-wrap as the old ShrinkWrap(clearance=0.0), so callers that pass no wrap_method keep the ball they had.
• generate_section_aero(solver, x, y) now delegates to generate_airfoil_aero instead of repeating the wrap-and-fit. This is the §2 dedup the card names.
• densify_contour moved instead of being copied: the ObjAdapter copy is deleted, build_section calls the AirfoilAero one, and the private docs list moved the entry. eltype(contour)[] handles both the tuple and the Vector callers.
• The warning reuses crossing_panels from #320 instead of adding a third crossing check. The card measures its cost.
• The new test would fail without the fix: it covers both radii the card reports as crossing and both deflections, and the card says 46 of 75 failed on the base branch.
• The card's pushback section discloses the follow-ups (#363, #364) and the polar shift on Kulfan inputs instead of leaving them out.

Not good

• src/airfoil_aero/airfoil_solvers/common.jl:110 — This builds ShrinkWrap positionally, so it silently depends on the field order clearance, min_concave_radius, min_clearance, n_points, curvature_weight. Adding or reordering a field would mis-assign values without any error. The keyword constructor at shrink_wrap.jl:41 already exists and names each value.
• src/airfoil_aero/shrink_wrap.jl:366 — The 0.01 edge cap is a bare magic number, and the docstring does not state it. Every closed input with edges over 0.01 is now densified, which is why Kulfan-derived polars move. The docstring should state the rule, or it should be a named constant.
• test/airfoil_aero/test_airfoil_aero.jl:303 — This @test_logs requires the default settings to cross on canopy[2]. That pins the known #363 defect as expected behaviour, so fixing #363 will break this test. It would be better to test the warning on a constructed crossing input that does not depend on the default mesh settings.
• The card lists obj_to_yaml polars shifting because Kulfan contours get densified as a risk that no test pins. That is a behaviour change to generated numbers and deserves its own CHANGELOG clause, not only the card.
• shrink_wrap now warns inside every deform_section, once per delta, so a delta sweep on the default mesh settings repeats the same warning n_delta times per section. That is noisy until #363 lands.
• Neither the docs build nor the local CI mirror finished before the PR opened. The densify_contour move changes the private docs list, which make.jl checks.
• Test comment at test_airfoil_aero.jl:294-295 describes the fix ("neither padded again nor collapsed"). Under §3 that belongs in the testset name or the PR, not in code.

_{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: This builds ShrinkWrap positionally, so it silently depends on the field order clearance, min_concave_radius, min_clearance, n_points, curvature_weight. Adding or reordering a field would mis-assign values without any error. The keyword constructor at shrink_wrap.jl:41 already exists and names each value.

[1-Bort-1]

Fixed in 7c53474: the re-wrap is built with the keyword constructor.

[1-Bort-1]

MINOR: The 0.01 edge cap is a bare magic number, and the docstring does not state it. Every closed input with edges over 0.01 is now densified, which is why Kulfan-derived polars move. The docstring should state the rule, or it should be a named constant.

[1-Bort-1]

Fixed in 7c53474: the shrink_wrap docstring and a Changed entry in the CHANGELOG now give the cap, min(0.01, min_concave_radius/2) chord.

[1-Bort-1]

MINOR: This @test_logs requires the default settings to cross on canopy[2]. That pins the known #363 defect as expected behaviour, so fixing #363 will break this test. It would be better to test the warning on a constructed crossing input that does not depend on the default mesh settings.

[1-Bort-1]

Fixed in 7c53474: the warning is now tested on a wavy membrane built to cross at clearance 0.05, and the same curve at clearance 0 checks that it does not warn. The test no longer uses V3 or #363.

[1-Bort-1]

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

[1-Bart-1]

Should merge into main, and be based on main

[codecov]

Codecov Report

❌ Patch coverage is 98.38710% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
ext/VortexStepMethodMakieExt.jl	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[1-Bort-1]

Fixed in ee7a6b3, which also closes #363: the default wrap no longer crosses on any V3 section and does not warn. Commented on 1-Bart-1/Agents#455.