Let a live polar refresh run without touching the heap - #270
Merged
Conversation
interpolate! hands back a gridded interpolation that reads the arrays it was given rather than copies of them, knots and values alike, which is what the rebuild in set_polar! was there to work around. Once a panel's interpolations read its own alpha_knots and coefficient vectors, writing those vectors is the whole update: a refresh of an unchanged shape allocates nothing and leaves the objects in place. A table that changes shape is still built once, and the two dimensional form fills every delta column of the matrix it already owns. The assignment back into the panel is guarded, since storing a returned immutable into a mutable field boxes it even when nothing changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
A refresh over 60 panels allocated 15.3 MB, nearly all of it inside NeuralFoil's forward pass: every layer built a fresh matrix for its product and another for its activation, the Mahalanobis distance sliced a column and did a matvec per case, and the symmetry embedding ran the whole thing again for the flipped inputs before combining the two into a third matrix. NeuralFoilWorkspace holds one set of layer activations per symmetry plus the flipped input batch, and fused_output! runs both passes through it. Bias and activation fold into one in-place sweep, the penalty is subtracted case by case straight onto the confidence logit, and the flip is undone while averaging rather than into a third matrix. flipped_row is now the one place the output mirror symmetry is written down. LivePolars caches the model and that workspace and preallocates every per-panel buffer a refresh touches, so shapes deform in place, the coefficients decode in place and the panel takes them as views. The surface-pressure pass shares the same workspace and reconstructs Cp through reused buffers. set_polar! had two leaks that only showed under real use: its getfield-by-symbol loop went dynamic once the four sources were no longer the same type, and storing live_shape boxes the immutable on its way into a Union field, which cost 48 bytes a panel even when the shape had not changed. refresh_live_polars! now allocates nothing, refresh_live_pressure! 96% less, and polar_drift, live_surface_friction! and live_shape_offset! nothing. Run time is unchanged; the forward pass is what is left. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0173NMo5G7SLMYjzCfr8L41u
As an immutable it was boxed on its way into the panel's live_shape Union field, so storing it cost 48 bytes a panel per refresh even when the shape had not changed. That needed an === guard to avoid, and the guard only worked because === on an immutable is structural: the panel was really holding a copy that compared equal to the shape it was sampled from. Mutable, the field holds the object itself. The guard goes, deform_kulfan! writes all four fields rather than requiring its target to already agree with the base on two of them, and a source that rewrites a shape in place has by that act updated every panel flying it. Two separately built KulfanParameters no longer compare === or == on identical contents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0173NMo5G7SLMYjzCfr8L41u
Single-core the refresh is arithmetic-bound and unchanged, so the changelog entry read as a wash. It is not: allocation is shared state and Julia's collector stops every worker, so a sweep running one model per core was losing half its wall time to GC. Eight concurrent workers go from 4.49 to 2.02 ms a refresh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0173NMo5G7SLMYjzCfr8L41u
On Julia 1.11 @allocated boxes a Float64 result, so the two new assertions read 16 bytes that refresh_live_polars! never allocated and failed CI there; 1.12 elides the box, which is why they passed locally. Reproduced on 1.11.9: measured returning the confidence, 16 bytes at top level and inside a let alike; measured returning nothing, 0 both ways. The closure had nothing to do with it. The two helpers now drop the confidence before the measurement, which is also why the set_polar! and live_surface_friction! assertions were green -- they measure calls returning nothing and a Vector. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0173NMo5G7SLMYjzCfr8L41u
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.
set_polar!rebuilt three interpolations per panel per refresh. It does not have to:interpolate!hands back a gridded interpolation that reads the arrays it was given rather than copies of them.I had this wrong earlier and said an upstream change was needed. It is
interpolate(non-bang), whichlinear_interpolationcalls, that copies the values;interpolate!aliases them. Verified on 0.16.3:(Knots were already held by reference either way — only the values forced the rebuild.)
Having got the panel down to zero, the rest of the live path was still allocating 15 MB a refresh, so the second commit takes that out too.
What changes
The panel's table (first commit)
rebuild_polarbecomesrefresh_polar!. Once a panel's interpolations read its ownalpha_knotsand coefficient vectors, writing those vectors is the update — the objects stay put and nothing is allocated. A table whose shape changes is built once, withinterpolate!over the panel's storage, and reads in place from then on.deltacolumn of the matrix it already owns rather than building a new one.reads_fromtells whether an interpolation's knot container is a view of the panel's angles or a copy — needed becauseScanKnotswraps the vector rather than being it.===. Storing a returned immutable into a mutable struct field boxes it even when the value did not change, which was the last 96 bytes.The network and everything around it (second commit)
Almost all of the remaining garbage was inside NeuralFoil's forward pass: every layer built a fresh matrix for its product and another for its activation,
squared_mahalanobis_distancesliced a column and did a matvec per case, and the symmetry embedding ran the whole thing again for the flipped inputs before combining the two into a third matrix.NeuralFoilWorkspaceholds one set of layer activations per symmetry plus the flipped input batch;fused_output!runs both passes through it. Bias and activation fold into one in-place sweep (add_bias!), the Mahalanobis penalty is subtracted case by case straight onto the confidence logit (penalize_confidence!), and the flip is undone while averaging (fuse_flipped!) rather than into a third matrix.flipped_rowis now the one place the mirror symmetry of the output layout is written down, shared byflip_outputsand the fusion.LivePolarscaches the model — the old code went through aDictkeyed by an interpolated string every refresh — and the workspace, and preallocates a buffer for every per-panel quantity a refresh touches. Shapes deform in place (deform_kulfan!, weights overwritten so the object a panel points at follows), coefficients decode in place (decode_coefficients!), and the panel takes them as views.set_polar!had two leaks that only showed under real use rather than in the first commit's test: itsgetfield/setfield!-by-symbol loop went dynamic once the four sources were no longer the same type (1104 B/panel), and storinglive_shapeboxed the immutableKulfanParameterson its way into theUnionfield (48 B/panel, even when the shape had not changed).KulfanParametersis now mutable, which removes that box. It also makes the claim honest: as an immutable,===is structural, so a panel'slive_shapewas really a copy that compared equal to the shape it was sampled from. Now it is the object. Two separately builtKulfanParametersno longer compare===or==on identical contents.Cpthrough reused buffers (contour_arc!,velocity_knots!,contour_pressure!, withsort_pairs!replacing twosortpermallocations).docs/make.jlraises Documenter'ssize_threshold: the new docstrings pushprivate_functions.htmlpast the 200 KiB default.Measured
60 panels × 9 samples (540 network cases,
xlarge), same script both sides:refresh_live_polars!refresh_live_pressure!live_surface_friction!live_shape_offset!polar_driftSingle-core run time is unchanged, which is the expected result rather than a disappointing one: allocation was never the bottleneck there. Profiled, a refresh is essentially all forward pass, and the pass splits into ~2.5 ms of BLAS
gemmand ~3.5 ms of bias-plus-swish— 691,200expcalls over ten 128×540 layer sweeps. Deform, input fill, decode and theset_polar!loop together are under 0.06 ms.Where it does pay is the way this is actually run: one model per core, sweeping. Allocation is shared state, and Julia's collector stops every worker. Same 60 panels, 20 refreshes per worker,
BLAS.set_num_threads(1):Throughput scaling over 8 cores goes from 2.2× to 4.1×, and the collector disappears from the run entirely. What is left at 8 workers is memory bandwidth, not garbage.
No threading was added inside the pass — the two symmetries are independent and spawning the flipped one measures 14.1 → 7.9 ms, but the model is meant to stay single-core so the sweep above it owns the cores.
The 146 kB left in the pressure path is the
FritschButlandMonotonicInterpolationobject, one per panel; removing it means reimplementing the monotone interpolation, which did not seem worth the risk on a function shared with the offline table generator.Test
test/airfoil_aero/test_live_polar.jlgains the properties both commits are for: after a refresh of the same shape the interpolation object is===what it was and@allocatedonset_polar!is 0, and@allocatedonrefresh_live_polars!— deformed and undeformed — and onlive_surface_friction!is 0 as well.Live polars 31 assertions, live pressure 8, friction 5, panel 24, plotting 58, plus the solver, wake and
test_results.jlsuites — all pass. That the numerics are untouched is pinned by three existing checks:test_results.jlagainst reference data, the livecpagainst the offlineNeuralFoilSolver's atrtol=1e-8, and the live polar against a directneuralfoil_aerocall atrtol=1e-6.Upstream context: JuliaMath/Interpolations.jl#656 asks for a selectable knot search, which is the other half of the lookup cost and the part that still has no API.
🤖 Generated with Claude Code