[1-Bort-1]

TL;DR

Adds unit tests for the two functions on #53's list that still exist. The smooth_circulation! test found a bug, fixed here: when an iteration's circulation was already smooth, damp kept the previous iteration's correction, so gamma_loop! kept applying it.

What happened to the three functions

• compute_lei_coefficients became compute_lei_coeffs (init! functions #73) and was then replaced by AirfoilAero.lei_poly_coeffs (Add neuralfoil and automatic conversion from obj to yaml #249). Existing tests only use it to build wings; nothing checked its values. The new test pins the coefficients at t = 0.07, κ = 0.095 to the output of the Python reference, AirfoilAero._instantiate_lei_airfoil_breukels_cl_cd_cm_coefficients in awegroup/Vortex-Step-Method@c82d521. I transcribed that function and ran it locally. It gives the same numbers as the Julia port, highest degree first. The test expects them in ascending order, so it also locks in the evalpoly order: without the final reverse! all 3 assertions fail.
• calculate_cosine_van_Garrel! was removed along with COSINE_VAN_GARREL in Unify RamAirWing and Wing into Wing #204, so there is nothing left to test.
• smooth_circulation! had no test and only runs when is_with_artificial_damping=true, which no test sets.

The bug

On both exits that skip smoothing, smooth_circulation! returned a new (zeros(n), false) tuple and never wrote to damp. Its only caller (gamma_loop!) ignores the return value and adds damp into gamma_new every iteration. So once smoothing had fired, every later smooth iteration re-applied that last correction. It showed up directly: a rough [0,1,3,1,0] left damp = [0, 0.389, -0.778, 0.389, 0], and a following call with the smooth [0,1,1,1,0] left damp unchanged.

Both exits now set damp .= 0 and return false, so the function always returns a Bool. The rewrite also folds the two exits into one condition and drops the comments that restated the code. It is otherwise unchanged: interior values move toward their neighbours' mean by damping_factor, then the result is rescaled to keep the total circulation.

Effect on a real solve: a 20-panel flat-plate LEI wing from test_solver.jl, LOOP/VSM with artificial damping on, 10 m/s. At 4° the circulation is bit-identical. At 16° CL goes from 1.4103 to 1.4983 and γ changes by up to 50%. Neither version converges at 16°, before or after the fix.

Found, not changed

• The smoothing threshold is smoothness_factor * mean(interior), which is signed. With a negative mean circulation the threshold is negative, so smoothing always fires. The Python original does the same.
• If the smoothed circulation sums to zero, the rescale divides by zero.
• The artificial_damping = (k2, k4) settings have fed nothing since calculate_artificial_damping was deleted in Delete the experimental functions calculate_artificial_damping()  #45; the solver hard-codes smooth_circulation!(damp, gamma, 0.1, 0.5). Removing or rewiring them changes public settings, so it is k2 and k4 in the artificial damping settings have had no effect since #45 #347.

Verification

• Reproduced first: after a rough call, a smooth call returned ([0.0, 0.0, 0.0, 0.0, 0.0], false) and left damp at [0.0, 0.389, -0.778, 0.389, 0.0]
• smooth_circulation! testset red before the fix (4 of 7 failed: the tuple return twice, the stale damp twice), green after (7/7)
• lei_poly_coeffs testset green 3/3 on unchanged code; 0/3 with the final reverse! removed
• test/solver/test_solver.jl 41/41, test/airfoil_aero/test_airfoil_aero.jl 90/90 (juliaserver, fresh session, test env)
• Local CI mirror (agent ci-local, Julia 1.12, full Pkg.test()): PASS in 8 min · GitHub CI: PASS (Julia 1.12 on Linux, Windows and macOS; 1.13 on Linux; docs; setup)
• Docs build: PASS in GitHub CI · REUSE: n/a, the repo has no bin/reuse_lint · 13 commits behind origin/main, merges cleanly (git merge-tree); none of them touch smooth_circulation! or poly.jl
• Risk: results with artificial_damping = true change wherever smoothing fired and later iterations were smooth, as intended. That includes the stall-corrected solver in examples/stall_model.jl, the only example or bundled YAML that turns it on, and nothing tests the damped solve end to end.

Scope

+46 / −33 across 4 files. src/solver.jl is net −14: the fix plus folding the duplicate exits. There are 27 lines of tests (two testsets) and a 5-line changelog entry under Fixed.

Closes #53 · task VortexStepMethod.jl-53

[1-Bort-1]

Independent review (advisory)

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

Good

• The bug fix is real: gamma_loop! (src/solver.jl:1067) adds damp into gamma_new every iteration, so the old exits that returned a tuple without writing damp did re-apply stale corrections, as I confirmed by reading the caller.
• The rewrite behaves the same apart from the fix: smoothed[i] += reads only circulation, || short-circuits so mean never runs on an empty interior, and the rescale is unchanged.
• The test expectation is correct: by hand, rough [0,1,3,1,0] with factor 0.5 gives smoothed [0,1.25,2,1.25,0], and rescaling by 10/9 gives damp [0,7/18,−7/9,7/18,0].
• The regression test works as the card claims: it calls rough then smooth on the same damp, so it fails on the old code (stale damp, tuple return) and passes after the fix.
• The skip path with no interior differences (length 3) is covered, and damp_short starts as ones, so zeroing is actually checked.
• The lei_poly_coeffs test pins values to a named external reference commit and also locks the ascending evalpoly order; the card says removing reverse! breaks all three assertions.
• Scope matches the plan: two tests from #53, one fix those tests found, the folded exits named as cleanup, and a changelog entry under Unreleased/Fixed. The dropped calculate_cosine_van_Garrel! is explained.
• The docstring now matches the actual Bool return and states what damp holds; smooth_circulation! is still listed in docs/src/private_functions.md.

Not good

• src/solver.jl:1122 — The only caller hard-codes smooth_circulation!(damp, gamma, 0.1, 0.5) (src/solver.jl:1072), and the artificial_damping=(k2, k4) setting is read into the solver but never reaches this function. Users tuning k2/k4 get no effect, and the card's survey of this function's quirks leaves this out.
• Local CI shows as 'started, not read' and GitHub CI is pending, so the full suite (including Aqua and docs) has not been seen green on this branch.
• smooth_circulation! still allocates interior, differences and smoothed on every solver iteration; views and a cached buffer would avoid that, but it predates this PR.
• The two 'Found, not changed' items (signed threshold with negative mean circulation, divide-by-zero when the smoothed sum is 0) are recorded only in the card; a follow-up issue would stop them getting lost after merge.
• Nothing tests a solve with damping on from start to end, and the card shows CL at 16° moving from 1.4103 to 1.4983, so a user-visible result shift is guarded only by the unit test.
• The caller's comments # Update gamma with relaxation and damping and # Apply damping if needed (src/solver.jl:1066,1070) just restate the code; they sit one step out and could have gone in the same tidy-up.

_{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 only caller hard-codes smooth_circulation!(damp, gamma, 0.1, 0.5) (src/solver.jl:1072), and the artificial_damping=(k2, k4) setting is read into the solver but never reaches this function. Users tuning k2/k4 get no effect, and the card's survey of this function's quirks leaves this out.

[1-Bort-1]

Confirmed: k2/k4 have fed nothing since calculate_artificial_damping was deleted in 0c6efde (#45). Removing or rewiring them changes public settings, so it's #347 rather than this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[1-Bart-1]

explain what was the problem and why it was fixed here

[1-Bort-1]

The problem: when smooth_circulation! decided the circulation was already smooth, it returned a new zeros(n) and never wrote to damp. So the damp buffer kept whatever correction an earlier iteration had put there, and gamma_loop! adds damp into gamma_new on every iteration (solver.jl:1067). With is_with_artificial_damping on, one rough iteration meant that correction was re-applied on every later iteration, even when no damping was needed. That also shifts the fixed point the loop converges to by damp / relaxation_factor.

Probe on the old code: rough [0, 1, 3, 1, 0] gives damp [0, 0.389, -0.778, 0.389, 0]. A following smooth [0, 1, 1, 1, 0] returns ([0, 0, 0, 0, 0], false) and leaves damp at [0, 0.389, -0.778, 0.389, 0].

Why it's fixed here: #53 asked for a unit test of this function, and that test is what exposed the bug. The fix is two lines inside the function under test: damp .= 0.0; return false on the skip path (dcaf811). It also makes every path return a Bool; the docstring promised a tuple, but the smoothing path already returned a bare true. Without the fix, a correct test of this function can only fail.

Who it affects: is_with_artificial_damping defaults to false, and every bundled settings YAML keeps it off, so default results don't change. The one example that turns it on is examples/stall_model.jl, for its stall-corrected solver at 17°, so that curve can move. With it on, a 20-panel flat-plate wing at 16° moves CL from 1.4103 to 1.4983. At 4°, where smoothing never fires, nothing changes.

[1-Bort-1]

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