Build a Solver from VSMSettings or from panel and section counts, deprecate the body_aero constructors, and check the sizes in solve!

[1-Bort-1]

TL;DR

Solver(settings) and Solver(n_panels, n_unrefined_sections) build a solver without a BodyAerodynamics. Solver(body_aero; kwargs...) and Solver(body_aero, settings) still work but are deprecated and warn on use, because they suggest the solver keeps the body. Since a solver can now be paired with any body, solve! checks the pairing up front and throws a DimensionMismatch naming both sizes. Before, a mismatch either failed deep inside the solve or returned silently wrong results.

What changed

Solver(n_panels, n_unrefined_sections, T=Float64; kwargs...) is the one constructor the others call. Solver(settings) sums n_panels over settings.wings and counts the wing_sections rows in each wing's geometry_file, which is the number of sections Wing(geometry_file) creates. Its solver_settings supply defaults, and keyword arguments override them.

Bart asked on the thread to keep the body_aero constructor but mark it deprecated. Both forms that take a body_aero now call Base.depwarn(...; force=true), so the warning shows even at the REPL's default --depwarn=no, once per call site. The message names the replacement. I deprecated Solver(body_aero, settings) too, since the issue is about the body_aero parameter and that form has it as well. Solver(settings) replaces it wherever the body was built from those settings.

Nothing in the package calls the deprecated forms any more:

• make_dual_shadow uses Solver(P, U, TD; ...), also for the several-wing body linearize and make_dual_shadow take a BodyAerodynamics with several wings #355 lets it take.
• Tests and examples with a body built from settings use Solver(settings). That includes examples/billowing.jl, which used to map solver_settings into keyword arguments by hand. data/TUDELFT_V3_KITE/vsm_settings_coarse.yaml named the 37-section aero_geometry.yaml, but it describes and the example flies the 10-section aero_geometry_coarse_discretisation.yaml, so it now names that file. Nothing else reads it.
• The rest build their wings in code with no settings file, and use Solver(wing.n_panels, wing.n_unrefined_sections; ...) with the same keyword arguments as before.
• examples/V3_neuralfoil.jl pairs a NeuralFoil wing with the CFD settings' solver fields. It now loads a second VSMSettings pointing at the generated YAML, as examples/V3_kite.jl already does.

The mapping from SolverSettings to keyword arguments was written out inline in Solver(body_aero, settings). It is now solver_kwargs, shared by both settings constructors. #352 added is_with_viscous_drag_correction to that inline mapping after this branch was opened; merging main moved it into solver_kwargs, so Solver(settings) carries it, and a testset in test/solver/test_viscous_drag_correction.jl fails if it is dropped. That file's own solvers now come from panel and section counts. The hasproperty(ss, :reference_point) branch is gone: SolverSettings has no such field, so it always fell back to the default.

The issue calls the second size n_groups. It is n_unrefined_sections here, the name Wing and the Solver{P, U, T} parameter already use.

How a mismatch failed before

I ran solve! on main with a solver one size off from the solver_test_wing body:

• One extra panel: DimensionMismatch: array could not be broadcast to match destination, raised partway through the solve.
• One extra unrefined section: no error. cl_unrefined_dist came back with one entry left at zero.

check_dimensions runs first in solve_base!, which solve!, solve and linearize all go through. It measured 0 bytes allocated.

Where I'd push back

Without a body_aero constructor, a body that wasn't built from settings means spelling out wing.n_panels, wing.n_unrefined_sections, and for several wings a sum over them. There is also no public way to give such a body the solver fields of a VSMSettings: examples/V3_neuralfoil.jl needed a second settings object for that. Solver(n_panels, n_unrefined_sections, settings) would close that gap. I left it out because nobody asked for it.

Solver(settings) reads each wing's geometry YAML a second time just to count rows. The alternative is an n_unrefined_sections field in WingSettings, but that is a second copy of a number that could drift from the file. A wing given only obj_file/dat_file gets an ArgumentError, as Wing(settings) does.

Found on the way, left for a cleanup: PR: .github/workflows/CI.yml:22 sets fail-fast: false.

Verification

• Reproduced first on main's src/ (outputs above)
• test/solver/test_solver.jl, sizes and new constructors: red on main's src/ ("3 passed, 0 failed, 4 errored"), green after
• test/solver/test_solver.jl, deprecation: red with the previous commit's constructors (17 passed, 2 failed: Log Test Failed, no captured logs), green after (19/19, whole file passes)
• Merges with main: test/solver/test_viscous_drag_correction.jl's new "solver settings switch it on" red with the field removed from solver_kwargs, green with it. After the second merge (28d4e11, Name the apparent wind va, va_vec and va_dist everywhere except the public API #349's va_vec rename), that file with test/solver/test_solver.jl, test/body_aerodynamics/test_results.jl and test/body_aerodynamics/test_body_aerodynamics.jl: 5048/5048, no warning in the output; mwes/mwe_warntype.jl runs
• Affected before the merges, all passing with no deprecation warning in the output:
  • test/body_aerodynamics/test_body_aerodynamics.jl
  • test/body_aerodynamics/test_results.jl 30/30
  • test/solver/test_flow_curvature.jl
  • test/solver/test_moment_units.jl 17/17
  • test/solver/test_forwarddiff.jl 7/7 (runs make_dual_shadow)
  • test/solver/test_unrefined_dist.jl 40/40
  • test/verification/test_verification.jl 22/22
  • test/solver/test_backend_comparison.jl 15/15
  • test/plotting/test_plotting.jl 58/58
• Examples run to the end, no warning: pyramid_model, rectangular_wing, stall_model, billowing, linearize_check, bench
• examples/billowing.jl on Solver(settings) (da666bf), run next to its previous version on Julia 1.13.0: both solvers Solver{54, 10, Float64}, all 20 solver fields equal, flat CL 0.6594671534084141, billowed CL 0.31720547039800795 and CD 0.059889001416727046 in both. The one visible difference is the legend, which reads "5.0%" instead of "5%" because billowing_percentage is a Float64 in WingSettings.
• Not run locally: test/bench.jl (its packages are in neither local environment; the suite runs it), and the V3_kite, V3_neuralfoil, ram_air_kite, obj_to_yaml_kite examples, which generate polars from meshes
• Third merge with main (12c11f4, Bound and trailing vortices share one segment kernel, so the trailing core induces azimuthal velocity #351 and linearize and make_dual_shadow take a BodyAerodynamics with several wings #355): the only conflict was CHANGELOG.md. The linearize_body test helper linearize and make_dual_shadow take a BodyAerodynamics with several wings #355 added called Solver(body_aero; ...), so it now sums the counts over the wings as solve_wings beside it does. On Julia 1.13.0: test/body_aerodynamics/test_body_aerodynamics.jl 4885/4885, test/filament/test_bound_filament.jl 225/225, test/solver/test_solver.jl 57/57, test/solver/test_forwarddiff.jl 7/7, no "deprecated" in the output
• Docs build clean (GitHub Documentation job on da666bf) · no REUSE in this repo · up to date with main at 086ee32
• Local CI mirror on 12c11f4 (Julia 1.13.0, one cell; da666bf changes only an example and a data file no test reads): PASS in 13 min, exit 0, and the word "deprecated" does not appear in the log
• GitHub CI on da666bf: PASS, all checks, after one re-run of Julia 1.12 - ubuntu. Before that, one Julia 1.12 cell per run failed the test_forwarddiff.jl's POLAR_MATRICES check flakes at ~4%, and the oper #287 check (test/solver/test_forwarddiff.jl:95). The PR run failed on ubuntu (0.046025390626216266), and a fresh workflow_dispatch on the branch failed on Windows (0.046430340012943265). main at 086ee32 fails the same check on its own push run, with that same Windows value bit for bit. Every failing cell built the NeuralFoil table whose norm_fd is 2.6237859245…. Every cell that built the other table (2.6186324991…) passes, with norm_fwd 2.618632581116621 on branch and main alike. A main dispatch passed all cells because its Windows runner built that other table. Not this diff; the evidence is on test_forwarddiff.jl's POLAR_MATRICES check flakes at ~4%, and the oper #287. The re-run built the 2.6186… table and passed. Earlier: 28d4e11 all jobs passed on the first attempt; d165c7f needed a second attempt at Julia 1.12 - ubuntu after the test_forwarddiff.jl's POLAR_MATRICES check flakes at ~4%, and the oper #287 flake (test/solver/test_forwarddiff.jl:87, 0.0420681631196805)
• Benchmark: n/a (one size check per solve, 0 bytes allocated)
• Risk: examples/V3_neuralfoil.jl's Solver(settings_nf) is not run here. If the generated YAML's section count differed from the wing's, solve! would throw DimensionMismatch rather than give wrong numbers.

Scope

+254 / −142 across 31 files against main.

• src/solver.jl +90/−29: the core constructor, the two deprecated methods, solver_kwargs (moved out of inline code, not new logic), n_unrefined_sections and check_dimensions.
• src/yaml_geometry.jl +12 counts sections from a WingSettings.
• test/solver/test_viscous_drag_correction.jl (from Add the Gaunaa et al. 2024 spanwise-flow viscous drag correction as an opt-in solver setting #352): off the deprecated constructor, plus the settings testset.
• examples/billowing.jl −44: its hand-written solver keyword mapping replaced by Solver(settings).
• The rest moves call sites in tests, examples, the mwe and docs/src/examples.md to the new constructors, plus the changelog. The changelog's two ### Fixed sections from the merge with main are now one.

Closes #153 · task VortexStepMethod.jl-153

[1-Bort-1]

Independent review (advisory)

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

Good

• The card's plan matches the diff: Solver(body_aero; ...) is kept, the new Solver(n_panels, n_unrefined_sections, T) is the one constructor the others call, and solver_kwargs is the inline mapping moved out unchanged (compared hunk by hunk)
• Dropping hasproperty(ss, :reference_point) changes nothing: src/settings.jl has no reference_point field on SolverSettings, so that branch always fell back to the default
• check_dimensions covers every entry point: solve! (solver.jl:314) and solve (solver.jl:611) both go through solve_base!, where the check is now the first line
• Counting wing_sections rows gives the same number as the built Wing: add_section! and refine! both set n_unrefined_sections = length(unrefined_sections), and normalize_span_order! only reorders sections
• Settings are splatted before the caller's kwargs, so keyword arguments override them, and the density=1.0 assertion tests exactly that
• The mismatch tests cover both silent-failure cases the card reproduced on main (one extra panel, one extra section), for solve! and solve, and check the message as well as the exception type
• Naming n_unrefined_sections instead of the issue's n_groups follows the existing Wing field and the Solver{P, U, T} parameter

Not good

• src/solver.jl:217 — Solver(settings) sums panels and sections over all settings.wings, but Wing(settings) builds only wings[1]. With a two-wing settings file (such as vsm_settings_dual.yaml once it has geometry files), the docs' Wing(settings) → BodyAerodynamics([wing]) → Solver(settings) pattern throws DimensionMismatch. Either use the same wing set in both, or say in the docstring that Solver(settings) expects a body built from every wing.
• n_unrefined_sections(::WingSettings) loads the geometry YAML a second time to get a count Wing(geometry_file) already derives; the card says so, but these are still two code paths for one number
• Inside Solver(n_panels, n_unrefined_sections, ...) the positional argument hides the module function n_unrefined_sections; harmless today, but a trap for the next edit to that body
• Line 203 of the constructor docstring ('and from its solver_settings the fields kwargs leaves unset') is hard to parse; one sentence per source of values would read better
• Nothing tests the ArgumentError for a wing without a geometry_file, and nothing tests Solver(settings) on a settings file other than solver_test_wing
• The card's 'found on the way' item, fail-fast: false in CI.yml, is a deliberate matrix setting, not a defect; it does not need a cleanup: PR
• The local CI mirror result was never read and GitHub CI has not run, so the verification is incomplete

_{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: Solver(settings) sums panels and sections over all settings.wings, but Wing(settings) builds only wings[1]. With a two-wing settings file (such as vsm_settings_dual.yaml once it has geometry files), the docs' Wing(settings) → BodyAerodynamics([wing]) → Solver(settings) pattern throws DimensionMismatch. Either use the same wing set in both, or say in the docstring that Solver(settings) expects a body built from every wing.

[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 (5 min, Julia 1.13.0, one cell of the matrix)

[1-Bart-1]

Mark the body_aero constructor deprecated (warn on use), but keep it.

[1-Bort-1]

Done in d165c7f: both Solver(body_aero; kwargs...) and Solver(body_aero, settings) still work but warn on use and name the replacement. Nothing in src, tests, examples or docs calls them any more.

[1-Bart-1]

why not use the settings-based constructor in the examples

[1-Bort-1]

Done in da666bf: billowing.jl now builds both solvers with Solver(settings). It couldn't before because vsm_settings_coarse.yaml pointed at the 37-section aero_geometry.yaml, while the example flies the 10-section coarse file, so Solver(settings) would have been the wrong size. The settings file now names the coarse geometry. Same 20 solver fields and bit-identical CL/CD as before. The examples still on counts (rectangular_wing, stall_model, bench, linearize_check, ram_air_kite, obj_to_yaml_kite) build their wings in code and have no settings file to read.