You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Objective. One results type: VSMSolution, filled by solve!, carries everything the solve() Dict carries; plotting moves onto it; then solve() and calculate_results go.
Approach — three stacked PRs, each green on its own.
Fill VSMSolution (this task). One big struct, per the thread. Fields keep the Dict names in lowercase: cl, cd, cs, lift, drag, side, cl_distribution, cd_distribution, cs_distribution, alpha_uncorrected, rey, q_ref, va_ref_vec, projected_area, area_all_panels, wing_span, aspect_ratio_projected. Fx/Fy/Fz, Mx/My/Mz, cfx.., cmx.. and F/M_distribution map onto the existing force, moment, force_coeffs, moment_coeffs, f_body_3D and m_body_3D. The expensive extras are center_of_pressure, panel_cp_locations and the projected distributions. They are Union{…, Nothing} and skipped unless a flag asks for them, so solve! keeps its allocation bound. calc_forces! and calculate_results compute every quantity through one shared function. That also removes a divergence found on the way: solve() applies correct_aoa for LLT too, and solve! does not. The test checks struct == Dict key by key on the existing test wings.
Remove solve() and calculate_results (BREAKING): the exports and docs entries go, the tests are ported to struct fields, and the CHANGELOG gets a BREAKING: line.
Still to decide.
Q1: PR 3 removes exported API. Recommendation: yes, as the issue says.
Q2: the skip flag. Recommendation: a solve! keyword, full=false, rather than a Solver setting, since plotting is the one caller that wants the extras. shortsolution=true, as proposed, is fine too.
Note: cl_distribution (projected on the inflow) sits next to the existing cl_dist (airfoil coefficient at the local alpha). The docstrings will say which is which. Renaming either one is out of scope.
Files.
PR 1: src/solver.jl, src/body_aerodynamics.jl, a new testset under test/solver/, CHANGELOG.md, the docs page for VSMSolution.
Taken into the review plan: field names are the Dict keys in lowercase (cl, cd, cs, lift, drag, side, cl_distribution, rey, q_ref, ...). Fx/Fy/Fz, Mx/My/Mz, cfx.. and cmx.. map onto the existing vector fields force, moment, force_coeffs and moment_coeffs.
Taken into the review plan: the expensive new fields (center_of_pressure, panel_cp_locations, the projected distributions) are Union{..., Nothing} and are skipped unless a solve! flag asks for them. The flag's name is still open there. PR 1 of the stack is the struct definition plus filling it.
All in one branch: c3c1fa7 fills VSMSolution and removes solve()/calculate_results; 5e56e6f makes calc_only_f_and_gamma skip the new fields. The PR opens from the branch at the end of this turn. Allocations (deterministic): calc_forces! 0 B → 0 B; solve! on a 40-panel rectangle 1760 B → 1760 B; solve! on the V3 kite 12672 B → 2080 B. Timing is inconclusive: I made four base→branch→base attempts pinned to one core, and the box was saturated every time (29–37 busy threads on 20 cores), so I claim no time ratio.
Activity
1-Bort-1 commentedon Sep 22, 2026
Review plan — #94 Remove results dict
Objective. One results type:
VSMSolution, filled bysolve!, carries everything thesolve()Dict carries; plotting moves onto it; thensolve()andcalculate_resultsgo.Approach — three stacked PRs, each green on its own.
cl,cd,cs,lift,drag,side,cl_distribution,cd_distribution,cs_distribution,alpha_uncorrected,rey,q_ref,va_ref_vec,projected_area,area_all_panels,wing_span,aspect_ratio_projected. Fx/Fy/Fz, Mx/My/Mz, cfx.., cmx.. and F/M_distribution map onto the existingforce,moment,force_coeffs,moment_coeffs,f_body_3Dandm_body_3D. The expensive extras arecenter_of_pressure,panel_cp_locationsand the projected distributions. They areUnion{…, Nothing}and skipped unless a flag asks for them, sosolve!keeps its allocation bound.calc_forces!andcalculate_resultscompute every quantity through one shared function. That also removes a divergence found on the way:solve()appliescorrect_aoafor LLT too, andsolve!does not. The test checks struct == Dict key by key on the existing test wings.plot_distribution,plot_combined_analysis,generate_polar_data/plot_polars, the examples anddocs/src/examples.md.Base.copy(::VSMSolution)is for result lists. It stacks on Render the README's V3 kite figure from a script with the current plot_combined_analysis, and fix that figure's layout #382, which editsplot_combined_analysis, if that is still open.solve()andcalculate_results(BREAKING): the exports and docs entries go, the tests are ported to struct fields, and the CHANGELOG gets aBREAKING:line.Still to decide.
solve!keyword,full=false, rather than a Solver setting, since plotting is the one caller that wants the extras.shortsolution=true, as proposed, is fine too.cl_distribution(projected on the inflow) sits next to the existingcl_dist(airfoil coefficient at the local alpha). The docstrings will say which is which. Renaming either one is out of scope.Files.
src/solver.jl,src/body_aerodynamics.jl, a new testset undertest/solver/,CHANGELOG.md, the docs page forVSMSolution.ext/VortexStepMethodMakieExt.jl,src/plotting_helpers.jl,src/VortexStepMethod.jl,examples/*.jl,docs/src/examples.md,test/plotting/test_plotting.jl.src/solver.jl,src/body_aerodynamics.jl,src/VortexStepMethod.jl,docs/src/{functions,tips_and_tricks}.md, the Dict tests,CHANGELOG.md.Remove the
agent:needs-plan-approvallabel to start.Task
VortexStepMethod.jl-94.1-Bart-1 commentedon Sep 24, 2026
Do all in one PR. Benchmark if this results in the
solve!getting significantly slower because it calculates more fields.1-Bort-1 commentedon Sep 24, 2026
Point 1 was ticked done rather than split out, so no new issue. The small-vs-large struct question is taken up in 2745201264: one struct now.
1-Bort-1 commentedon Sep 24, 2026
Taken into the review plan: one big VSMSolution. A smaller struct is left for a later issue.
1-Bort-1 commentedon Sep 24, 2026
Taken into the review plan: field names are the Dict keys in lowercase (cl, cd, cs, lift, drag, side, cl_distribution, rey, q_ref, ...). Fx/Fy/Fz, Mx/My/Mz, cfx.. and cmx.. map onto the existing vector fields force, moment, force_coeffs and moment_coeffs.
1-Bort-1 commentedon Sep 24, 2026
Taken into the review plan: the expensive new fields (center_of_pressure, panel_cp_locations, the projected distributions) are Union{..., Nothing} and are skipped unless a solve! flag asks for them. The flag's name is still open there. PR 1 of the stack is the struct definition plus filling it.
1-Bort-1 commentedon Sep 24, 2026
Taken into the review plan: all new fields are lowercase (rey, not Rey).
1-Bort-1 commentedon Sep 24, 2026
All in one branch: c3c1fa7 fills VSMSolution and removes solve()/calculate_results; 5e56e6f makes calc_only_f_and_gamma skip the new fields. The PR opens from the branch at the end of this turn. Allocations (deterministic): calc_forces! 0 B → 0 B; solve! on a 40-panel rectangle 1760 B → 1760 B; solve! on the V3 kite 12672 B → 2080 B. Timing is inconclusive: I made four base→branch→base attempts pinned to one core, and the box was saturated every time (29–37 busy threads on 20 cores), so I claim no time ratio.