Bound and trailing vortices share one segment kernel, so the trailing core induces azimuthal velocity

[1-Bort-1]

TL;DR

velocity_3D_trailing_vortex! and velocity_3D_bound_vortex! were two copies of the same Biot–Savart segment formula, differing only in how the core radius is picked. #241 fixed the in-core projection in one copy; both now pick their core radius and call one kernel, velocity_3D_vortex_segment!, so the trailing core induces azimuthal velocity like the bound one.

What was wrong

Inside its core the trailing vortex moved the field point onto the core boundary along r1 × r0, which points azimuthally. The Biot–Savart velocity at that point is radial. Reproduced on main with the issue's setup (filament (0,0,0)→(1,0,0), Γ = 1, va = 1e-4, point at x = 0.5, distance r along y):

before  r=0.05 vel=[0.0,  0.01357, 0.0]      after  r=0.05 vel=[0.0, -0.0, 0.01357]
before  r=0.1  vel=[0.0,  0.02713, 0.0]      after  r=0.1  vel=[0.0, -0.0, 0.02713]
before  r=2.0  vel=[0.0, -0.0,     0.0193]   after  r=2.0  vel=[0.0, -0.0, 0.0193]

The magnitude was already right; only the direction changes.

Why one kernel rather than patching the second copy

The bug existed because a fix reached one of two copies. Apart from the core radius (core_radius_fraction · |r0| for bound, the Oseen sqrt(4 α₀ ν |r1·r0|/|r0| / va) for trailing), the in-core and out-of-core branches were the same formula. Now the two public functions are three lines each and the formula exists once. src/filament.jl is 56 lines shorter.

Checked against a copy of main's filament.jl loaded in the same session, over 20 000 random filaments, points and va from 1e-5 to 10:

• trailing, outside the core (8 273 points): bit-identical, max |Δ| = 0.0
• trailing, inside the core (11 727 points): |v| agrees to a relative 1.7e-12. Only the direction changed.
• bound (20 000 points): max |Δ| = 3.6e-15, from reordering one product

The @debug lines are gone

The bound kernel logged two @debug messages in its in-core branch. In the shared kernel they made the trailing in-core path ~2.5× slower (35.9 → 92 µs per 1000 calls, in one run whose conditions held), so I removed them. They only printed at debug level.

Core radius: axial or perpendicular distance

The issue asks which is right. I'd keep the axial distance and changed nothing here. |r1·r0|/|r0| is how far along the trailing filament the point sits, i.e. the vortex age t = s / va in Lamb–Oseen's r_c = sqrt(4 α₀ ν t). Measuring from the perpendicular distance d instead gives ε = sqrt(c·d) with c = 4 α₀ ν / va, so the in-core test d < ε becomes d < c: a fixed core of 7.4 µm at 10 m/s that never grows along the wake, and inside it the velocity scales with sqrt(d/c) instead of linearly. awegroup/Vortex-Step-Method@ed6f158 made that switch as a side effect of reusing the name r_perp for the radial vector. Its commit message argues only for the projection, not the core radius. What was wrong here was the comment, which called the axial length "the perpendicular component". The trailing function now names that length axial_distance, and the semi-infinite kernel's comment says "axial distance".

Verification

• Reproduced first: trailing r=0.05 vel=[0.0, 0.01357, 0.0] (all radial inside the core)
• test/filament/test_bound_filament.jl: the azimuthal testset loops over both velocity_3D_bound_vortex! and velocity_3D_trailing_vortex!, each at six radii (0.25–2 × its core radius) and five angles. Red against the pre-fix filament.jl (trailing 70/90: the four in-core radii × five angles fail), green after (225/225)
• After merging main (the Name the apparent wind va, va_vec and va_dist everywhere except the public API #349 v_a → va rename was the only conflict, in src/filament.jl), green in a fresh juliaserver session: filament/test_bound_filament.jl 225/225, filament/test_semi_infinite_filament.jl 58/58, panel/test_panel.jl 26/26, verification/test_verification.jl 22/22, body_aerodynamics/test_body_aerodynamics.jl 4870/4870. Before the merge also solver/test_solver.jl all, solver/test_forwarddiff.jl 7/7, bench.jl allocation tests 21/21
• Docs build clean (local makedocs, no deploy) · up to date with main · REUSE: n/a (repo does not use it)
• GitHub CI on 3dc74fc: PASS, all five cells, the setup check and codecov/patch. The first attempt of the 1.12 ubuntu cell failed test/solver/test_forwarddiff.jl:87 at 0.0460232000169766 < 0.0001 (norm_fwd = 3.2832728432488474, norm_fd = 3.285180977112122). main at 83867a3, this branch's merge base, failed that cell with the same three numbers to the last digit (run 35197849038), and the cells that passed show the same norms as main's passing cells. That makes it flake test_forwarddiff.jl's POLAR_MATRICES check flakes at ~4%, and the oper #287, not this branch. The re-run passed and no code changed. The run on 30b0756 hit the same flake and also passed on re-run
• Local full suite (agent ci-local, Julia 1.12.7) on 3dc74fc: PASS in 8 min, 6638 passed, 1 broken (the existing @test_skip in test/ram_geometry/test_kite_geometry.jl)
• Benchmark: per-call medians, 1000 points, main's kernels vs this branch interleaved in one session, two runs whose conditions held, 0 allocations on both sides. Outside the core, where a solve evaluates: bound 17 → 17 ns, trailing 27 → 28 ns. Inside the core: bound 92–97 → 35–38 ns (the @debug lines), trailing 35–37 → 39–40 ns (+10%, the radial vector it now forms). Isolated 2× spikes on both sides are left out of these ranges.
  Box: 11–12 of 20 cores busy, 2.8–3.6 GHz, ≥20 GB free
• Risk: velocity_3D_trailing_vortex! now forms r1 twice per call, once for the core radius and once in the kernel. The outside-core benchmark above is the check on that.

Scope

+78 / −123 across 4 files. src/filament.jl is the kernel extraction plus the comment rewrites. test/filament/test_bound_filament.jl extends the existing azimuthal testset to the trailing vortex and imports ALPHA0, NU. CHANGELOG.md gets one Fixed line, and main's two Fixed headings under Unreleased become one (tidy-up, no entry changed). The new private function is listed on docs/src/private_functions.md.

Closes #333 · task VortexStepMethod.jl-333

[1-Bort-1]

Independent review (advisory)

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

Good

• One source per equation: both public functions now only choose epsilon and call velocity_3D_vortex_segment!, so the Biot–Savart formula exists once; checked by reading src/filament.jl on the branch
• The fix is correct: the in-core branch now puts the point on the core boundary along the radial vector r1 - (r1·r0)r0/|r0|². The old trailing code used r1×r0 (and r2×r0, the same direction), which is azimuthal; checked against the removed lines
• No new aliasing: the kernel now uses work_vectors[1..7] where the old trailing code used [2..8], but the only src callers (panel.jl:682-703) pass evaluation_point, a panel field, never a work vector
• Unifying the two copies instead of patching the second is what §2 asks for, and the card gives the reason and the parity numbers (bit-identical outside the core, 3.6e-15 for bound)
• The test fails before the fix (15 failures = 3 in-core radii × 5 angles) and passes after. Its radii 0.99/1.01 × core radius cover both sides of the branch switch
• Docstrings fixed as a side benefit: the missing ! in both signatures, trailing whitespace, and each wrapper now says in one line how it picks its core radius
• The changelog line states the user-visible change and its millimetre-scale reach; the new private function is listed in private_functions.md, so makedocs stays green
• The axial-vs-perpendicular core radius question is argued in the card and left unchanged in code, as the plan said; axial_distance now names what the old comment wrongly called perpendicular

Not good

• test/filament/test_bound_filament.jl:255 — This testset nearly copies 'Velocity is azimuthal (perpendicular to axis and radius)' at line 179, changing only the function called and the core radius. Since both functions now share one kernel, one loop over (function, core radius) pairs would state the invariant once instead of keeping two copies in step (§2, §7).
• velocity_3D_trailing_vortex! writes r1 into work_vectors[1] and the kernel recomputes it right away; this costs ~1 ns and is named in the card, but it is a wasted line in the hot loop
• src/filament.jl:210: the rewritten semi-infinite comment mostly restates epsilon = sqrt(... abs(d_r1_Vf) * nVf / v_a); by the §3 deletion test it could go
• The card says 'the comments in both trailing kernels now say axial distance', but in the finite kernel it is now a variable name, not a comment. The card is slightly off; the code is fine
• The new testset checks direction only. Magnitude and solid-body scaling for the trailing path are covered only indirectly, through the bound tests on the shared kernel
• Test locals p, v, d are one-letter names (§3); they copy the neighbouring test, and r0 = [1.0, 0.0, 0.0] repeats filament.r0
• Removing the two @debug lines is a small behaviour change outside the bug fix; it is named and measured in the card, so it is acceptable, but it is not in the changelog

_{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 testset nearly copies 'Velocity is azimuthal (perpendicular to axis and radius)' at line 179, changing only the function called and the core radius. Since both functions now share one kernel, one loop over (function, core radius) pairs would state the invariant once instead of keeping two copies in step (§2, §7).

[1-Bart-1]

Fix this

[1-Bort-1]

Fixed in 465d199: one testset now loops over both vortex functions, each at its own core radius.

[1-Bort-1]

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!