Fix branch 3 direction#241
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a bug in the Lamb-Oseen-style core regularization (Branch 3 of
velocity_3D_bound_vortex\!andvelocity_3D_trailing_vortex_semiinfinite\!). The projection onto the core boundary was being done in the azimuthal direction (r1×r0 / |r1×r0|) instead of the radial direction. This made the regularized induced velocity inside the core point in the wrong direction — typically perpendicular to where it should point.The bug is pre-existing and shared with the Python implementation (
Vortex-Step-Method/src/VSM/core/Filament.py); both inherit it from the original KiteAeroDyn-style implementation referenced as Damiani et al. The Julia tests didn't catch it because they were calibrated under the bug. The Python tests all setcore_radius_fraction = 1e-20, which makes Branch 3 effectively never fire, so the verification suite never exercised the regularization code path.The fix
A vortex's induced velocity is purely azimuthal (perpendicular to both the axis and the radial direction) — this is a direct consequence of Biot-Savart and axial symmetry. Inside the core, the standard regularization (Rankine / Scully / Lamb-Oseen profiles) projects the field point radially outward onto the core boundary, evaluates the inviscid Biot-Savart velocity there, and scales by
d_perp / epsilon(linear ramp from zero on axis to peak at boundary).The fix replaces the azimuthal projection direction with the actual radial direction — the perpendicular component of
r1to the filament axisr0:Same fix in both
velocity_3D_bound_vortex\!andvelocity_3D_trailing_vortex_semiinfinite\!.References for the radial-projection convention:
Tests
New tests on
test/filament/test_bound_filament.jlthat exercise the regularization code path properly:d_perp, directions stay parallel along a radial line.v_θ = Γr/(2π·r_c²)for a long filament. Independent of the rest of the panel method.Equivalent direction-check test added on
test_semi_infinite_filament.jl.Updated existing tests where the old fixtures encoded the bug:
induced_velocity[3] ≈ 0, but the azimuthal direction is±zfor this geometry, so that component must be the only nonzero one. Updated to assert axial/radial components are zero and the azimuthal component is nonzero.Notes / follow-ups
d_perp/epsilonlinear ramp that the bound vortex Branch 3 does — it returns the peak velocity throughout the core. Looks unintentional (and inconsistent between the two filament functions) but is out of scope here. The new "Constant azimuthal direction inside core" test for the semi-infinite filament only asserts direction continuity inside the core, not magnitude.Vortex-Step-Method/src/VSM/core/Filament.py) has the identical bug in bothvelocity_3D_bound_vortexandvelocity_3D_trailing_vortex, plus a closely-related (but distinct) issue invelocity_3D_trailing_vortex_semiinfinitethat uses anr1/|r1| - Vfapproximation instead of the exact radial direction. Worth a parallel PR upstream.Test plan
julia --project=. -e 'using Pkg; Pkg.test()'locally