plot_section_polars draws cl, cd and cm against alpha per panel, for every aero model - #354
Merged
1-Bart-1 merged 5 commits intoSep 20, 2026
Merged
Conversation
plot_section_polars(body_aero; panels, alphas, delta) evaluates each chosen panel through calculate_cl/cd/cm and draws the three coefficients in one MakieControlPlots figure, so POLY, INVISCID, POLAR_VECTORS and POLAR_MATRICES share one path. Replaces the POLAR_VECTORS-only, one-coefficient-per-call form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
panel_polar_curves evaluates calculate_cl/cd/cm over an alpha grid; both plot_section_polars and the POLAR_VECTORS panel of plot_combined_analysis use it instead of calling the raw interpolants. Split the plot_section_polars test into one testset per aero model and reuse create_body_aero for the POLY wing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1-Bort-1
commented
Sep 16, 2026
1-Bort-1
left a comment
Member
Author
There was a problem hiding this comment.
Independent review (advisory)
Verdict: APPROVE WITH COMMENTS · 1 inline, 0 off the diff
Good
- The plot now reaches every aero model:
calculate_cl/cd/cminsrc/panel.jl:504-580handle POLY, INVISCID, POLAR_VECTORS and POLAR_MATRICES, and usedeltaonly for POLAR_MATRICES, as the docstring says. - One code path, as the card promises:
panel_polar_curvesreplaces the three raw*_interpcalls in the POLAR_VECTORS branch ofplot_combined_analysis, so no third copy exists. - Panels and deflections pair up correctly:
calculate_cl.(panels, alphas', deltas)with a per-paneldeltasvector gives an n×m grid,something.(nothing, …)works because broadcasting treatsnothingas a scalar, andvcat(panels)accepts both an Int and a range. labels=[labels]fitsMakieControlPlots.plotx(plotx.jl:27-37): the panel legend goes only on the first (cl) row, which is right for three rows sharing an α axis.- The tests pin each model to a closed form (2πα, the linear table,
evalpolyof the Breukels coefficients, the POLAR_MATRICES interpolant at the passed delta) and do not just check that a figure exists; they reusecreate_body_aerothrough new keywords, not a new fixture family. - The BREAKING change is small: a search finds only the one example caller, updated in this diff, plus the changelog and docs stubs; the new helper is listed in
private_functions.md. - The scope matches the card: six files, the combined-analysis tidy-up is named, and the found problems (POLY jump at 20°, transposed wireframe) are noted, not fixed silently.
Not good
ext/VortexStepMethodMakieExt.jl:1521— By defaultpanelsis every panel, so a normal 20–40-panel wing draws 20–40 overlapping curves and a legend with that many entries on the cl row; the one example caller already overrides it with[1, 10], which suggests the default is not a useful figure.- The title is still "Section polars" and the file
section_polars.png, but the figure now shows panel polars (averaged over the refined panels); the function name is API, but the title string could say what is drawn. - The two found problems (POLY discontinuity at |α| = 20°, transposed POLAR_MATRICES wireframe in
plot_combined_analysis) have no issue yet; they are left to the human, where §5 says to open issues and link them from the PR. - The wireframe transposition is in the same
ifblock this PR edits, and fixing it changes only which axis is labelled what; it could arguably have gone into this diff, not just been noted. - The CI matrix
fail-fast: falsebullet does not say what is wrong with it, so a reader cannot tell why it is listed under found problems. - Local CI and GitHub CI are both still pending in the card, so the only green result so far is from the juliaserver session.
- The
panel_polar_curvesdocstring allows "one shared deflection" only for theplot_combined_analysiscaller;plot_section_polarsalways passes a vector, so the helper supports two argument shapes for two callers.
claude, rubric CLEAN_CODE.md. A different lab from the implementer
on purpose: a reviewer sharing its blind spots would not flag its mistakes.
| end | ||
| isempty(series) && error("No POLAR_VECTORS sections found in body") | ||
| function VortexStepMethod.plot_section_polars(body_aero::BodyAerodynamics; | ||
| panels=eachindex(body_aero.panels), alphas=deg2rad.(-20:0.5:30), delta=nothing, |
Member
Author
There was a problem hiding this comment.
MINOR: By default panels is every panel, so a normal 20–40-panel wing draws 20–40 overlapping curves and a legend with that many entries on the cl row; the one example caller already overrides it with [1, 10], which suggests the default is not a useful figure.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
1-Bart-1
requested changes
Sep 17, 2026
…n-polars-covers-only-polar-ve
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
Author
|
Local full suite: PASS (6 min, Julia 1.13.0, one cell of the matrix) |
…n-polars-covers-only-polar-ve # Conflicts: # CHANGELOG.md
1-Bart-1
approved these changes
Sep 20, 2026
1-Bart-1
deleted the
agent/331-plot-section-polars-covers-only-polar-ve
branch
September 20, 2026 16:13
This was referenced Sep 20, 2026
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.
TL;DR
plot_section_polars(body_aero; panels, alphas, delta)now draws cl, cd and cm in one figure, one curve per chosen panel, through the panel's owncalculate_cl/calculate_cd/calculate_cm. Before, it drew one coefficient per call from rawPOLAR_VECTORSsection data only. APOLY,INVISCIDorPOLAR_MATRICESwing gaveNo POLAR_VECTORS sections found in body, and nothing showed what a panel uses at a given flap deflection (#113).What changed
panel_polar_curves(panels, alphas, deltas), runs the samecalculate_*functions the solver calls over an α grid. The plot therefore shows what the solver sees: the averaged, interpolated panel polar, with its extrapolation andalpha_window. It no longer shows the unrefined section's CSV points.delta[rad] sets the deflection aPOLAR_MATRICESpanel is evaluated at. It defaults to each panel's storeddelta, and other models ignore it.panelstakes one index or a list and defaults to all panels.alphasis in radians, likeplot_polar_data's. The default −20°…30° is the range of the Pythonplot_panel_coefficients.MakieControlPlots.plotxwith three rows on a shared α axis.coefficientargument is gone, soplot_section_polars(body_aero, :cd)now raises aMethodError; its one caller,examples/obj_to_yaml_kite.jl, now makes a single call.POLAR_VECTORSpanel ofplot_combined_analysisused to call the rawcl_interp/cd_interp/cm_interpfor the same three curves; it now uses the same helper. The two paths differ only wherealpha_window > 0, which the raw interpolants skipped. Oncreate_body_aero_with_skinover −5…20° the old and new values are identical (alpha_window = 0).There is no before figure. Old code raised an error on the POLY wing, and on the V3 kite it plotted different data: raw section points, not panel polars.
Found on the way (not changed here)
POLYis discontinuous at |α| = 20°, which the plot shows at once: atlei_poly_coeffs(0.1, 0.08)cl drops from 1.0706 to 0.2200 across 20°, and cm's quadratic reaches −5.53 at 30°. Fixing it changes existingPOLYresults, so it is POLY cl and cd jump at |alpha| = 20 deg, and cm runs its quadratic unbounded past it #359.plot_combined_analysisdraws itsPOLAR_MATRICESwireframes transposed.for alpha in alphas, delta_te in delta_tes, which puts α on the rows. It is then drawn withwireframe!(ax, delta_tes, alphas, M), which expects δ on the rows.plot_polar_databuilds the matrix the other way round and is correct.-5:0.3:20, so nothing errors. I'd fix it by having both functions share one wireframe helper; that changes a figure, so it is not in this diff..github/workflows/CI.yml:22carriesfail-fast: false, so one failing cell does not cancel the others and a red run costs the whole matrix; turning it off is acleanup:PR of its own.Verification
plot_section_polars(create_body_aero(), :cl)(INVISCID) andplot_section_polars(BodyAerodynamics([ram_wing]), :cl)(POLAR_MATRICES) both raiseNo POLAR_VECTORS sections found in body.test/plotting/test_plotting.jlwas red before the change (MethodError: no method matching plot_section_polars(...; panels, alphas)) and is green after, 19/19. It has one nested testset per behaviour:evalpolyof the Breukels coefficients;delta = 1°matches the interpolants there and differs from the storeddelta;test/plotting/test_plotting.jlpass in the juliaserver session: Plotting 58/58, the new testset 19/19, Airfoil skin 22/22, generated_slices 6/6, Audit slices 7/7.plot_combined_analysison a POLAR_VECTORS body still builds aFigure(checked by hand; no test covers that branch).docs/make.jl) is clean apart from the existing size-threshold warnings. The repo has no REUSE.origin/maintwice, most recently at 7103702 (after Name the apparent wind va, va_vec and va_dist everywhere except the public API #349'svarename, 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 the changelog's Added list, where both entries are kept.test/plotting/test_plotting.jlwas re-run after each merge, and all testsets pass as above, now on Julia 1.13.0.plot_section_polars(body_aero, :cd)break. The old form had no test and one example caller.Scope
+126 / −53 across 6 files against main:
ext/VortexStepMethodMakieExt.jl: the function body, now shorter, plus the helper and the three lines inplot_combined_analysis.Searched for
calculate_cl,cl_interp(,plot_polar,panel_coefficientsandalpha_rangeinsrc/andext/. The only other code that evaluates a panel over an α grid isplot_combined_analysis, which is folded in above.Closes #331 · task
VortexStepMethod.jl-331