feat(memory): size the LoRA prefill scratch, not just adapter storage - #6
Open
dittops wants to merge 8 commits into
Open
feat(memory): size the LoRA prefill scratch, not just adapter storage#6dittops wants to merge 8 commits into
dittops wants to merge 8 commits into
Conversation
Serving a LoRA-enabled model allocates two very different things, and this
library reported only the small one.
`calculate_lora_adapter_memory` returns the adapter A/B *storage*: 0.22 GiB at
rank 256 on a 0.6B model. Punica's prefill working buffers at the same rank are
16.77 GiB -- 75x larger -- and were reported as nothing at all. A control plane
sizing a pod off the first number under-sizes it by that factor and the pod
OOMKills at "Warming up model for the compilation", which destroys the evidence
along with the pod.
Add `calculate_lora_prefill_scratch`, reported as its own `MemoryReport` field
rather than folded into the storage term: summed together it would read as a
rounding error on a number readers already believe is small.
Calibration (Qwen3-0.6B, 28 layers, rank 256, max_num_seqs=1, vLLM CPU, cgroup
limit 40 GiB so nothing was clipped). LoRA-attributable is the LoRA-on peak minus
a LoRA-off control at the identical token budget:
tokens | LoRA off | LoRA on | scratch
-------|-----------|-----------|------------
2394 | 5143 MiB | 22311 MiB | 16.77 GiB
4273 | 5113 MiB | 27119 MiB | 21.49 GiB
8192 | 5096 MiB | OOM @50G | > 45.02 GiB
Two results worth stating. Base memory is FLAT in the token budget -- 47 MiB
across a 3.4x range -- so every bit of a LoRA pod's context sensitivity is this
one term. And the term is sized by `max_num_batched_tokens`, the tokens in one
forward pass, NOT by the context length: with chunked prefill those differ, and
charging the context prices an 8192-context pod at 31 GiB it never allocates.
`max_num_batched_tokens` threads through `calculate_memory` and defaults to
`seq_length`, so callers that do not set it get the unchunked reading.
The fit is honest about where it stops. Above 4273 batched tokens the term goes
convex -- at 8192 the linear form under-predicts a measured floor by >13 GiB --
so it returns a note marking the figure a LOWER BOUND rather than an estimate. It
also notes when a request leaves the measured 28-layer/rank-256 geometry, since
linearity in depth and in rank is assumed, not measured.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
A rank-64 probe (2394-token budget, same image/model as the rank-256 calibration) peaked 2.82 GiB above its LoRA-off control, where `units` predicts 16.77/4 = 4.19. Four times the rank costs 5.95x the scratch -- about rank^1.29, not linear. Which direction that errs in is the part a caller has to know: * Below rank 256 the linear form OVER-charges. Conservative; the probe ran at 61% of the limit sized from it, 0 restarts. * Above rank 256 it UNDER-charges, and that is an OOMKill at warmup. At rank 512 it gives 33.5 GiB where the power law gives 40.9 -- 7.4 GiB short. Both are now reported through `notes`, rather than one number with no indication of which side of the calibration it sits on. The fit itself is unchanged: refitting the rank exponent from two points, when the token slope was itself only fitted at rank 256, is the same over-reach that produced two earlier wrong models here. The depth note is narrowed to depth alone and no longer implies rank is untested -- rank now has two measurements, depth still has one, and rank turning out superlinear is reason to trust assumed depth linearity less rather than more. Testing: pytest tests/test_lora_prefill_scratch.py -q -> 15 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two corrections, both from same-series measurements (LoRA-on minus a LoRA-off control run minutes apart, cgroup `anon` rather than memory.peak so page cache cannot inflate it, limits generous enough that nothing clipped). 1. The token slope steepens at ~4273 batched tokens. Rank 64: 5.14 GiB at T=4273, 10.39 GiB at T=7310 (repeat 10.38, so the term is deterministic to ~1 MiB). A single slope fitted below the knee under-predicts T=7310 by 3.10 GiB -- 43% -- and that is not academic: a production pod sized on it sat against its cgroup ceiling, hit the limit 196 times, and stayed up only by evicting page cache. The piecewise form is within 2-4% and conservative at both points. Cross-check: extending the steep segment to T=8192 at rank 256 predicts 48.6 GiB of scratch, and such a pod did OOMKill at a 50 GiB limit. Consistent. 2. The calibrated rank drops from 256 to 64, because 64 is the only rank actually characterised. The rank-256 figures came from max_num_seqs=1 pods; at rank 64 the term is seqs-independent (1 vs 2 differ by 54 MiB), which made those look generalisable. They are not -- a rank-256 pod at T=4273 with seqs=2 OOMKilled at a 55 GiB limit, where linear-in-rank predicts 20.6 GiB. WITHDRAWN: the previous commit claimed rank scales as ~rank^1.29. That came from comparing pods across two campaigns with different controls. Same-series data puts rank 64 and 256 within 4.5% of linear at T=4273, so the exponent is not supported and the claim is removed rather than restated. What is known is narrower: rank 64 is characterised, rank 256 is not, and above the ceiling the error is an under-estimate of unknown size. Also withdrawn: the "~1.6 GiB run-to-run variance" reading. That was vLLM's instantaneous free-memory probe, which allocator retention perturbs; the term itself repeats to 1 MiB. Testing: pytest tests/test_lora_prefill_scratch.py -q -> 16 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A note is not protection. The previous commit reported an un-margined figure past the calibrated ceiling and labelled it a LOWER BOUND. A T=17600 deployment -- 2.4x past calibration -- shipped on that figure and crash-looped: vLLM's KV check found 8.8 GiB free where it needed 10.0, short by 1.21 GiB, identically across 5 restarts. The warning fired and the pod died anyway. Beyond LORA_SCRATCH_CALIBRATED_MAX_TOKENS the term now carries a 20% margin. The one measured extrapolation error is +4.3% of the extrapolated term; 20% is about 4.6x that, chosen because the two failure directions are not symmetric -- too much costs memory on a machine with plenty, too little costs a crash-loop. Calibrated deployments are unaffected: the multiplier applies only above the ceiling. At T=17600 this takes the term from 28.41 to 34.09 GiB, clearing the >=29.62 GiB the failure showed was needed. Testing: pytest tests/test_lora_prefill_scratch.py -q -> passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
a4a1bb7 changed the past-ceiling note from "LOWER BOUND" to reporting the applied margin, but left test_flags_uncalibrated_token_budget asserting the old wording. That commit's message claimed the suite passed. It did not -- the test run and the commit were not gated on each other, so the failure was recorded as a pass. The suite is green here: 17 passed. The replacement also asserts the margin as arithmetic rather than wording, and pins the T=17600 production config that crash-looped (must now exceed the 29.62 GiB the failure showed was required). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ys otherwise
`_is_down_shared` inferred that all experts share ONE down-projection whenever
`n_routed_experts > 32` and `moe_intermediate_size / hidden_size < 0.6`, commented
"small experts often share down projection".
That inference is wrong, and wrong in the direction that under-counts a model into
an OOMKill. A small intermediate/hidden ratio is the SIGNATURE of fine-grained MoE
-- many narrow experts -- not evidence of weight sharing. Every mainstream MoE gives
each expert its own gate, up and down matrix.
It fired on exactly the architectures that do not share, and Mixtral escaped only by
having 8 experts:
Qwen3.5-35B-A3B 256 experts, ratio 0.250 -> fired
DeepSeek-V3 256 experts, ratio 0.286 -> fired
Qwen3-235B-A22B 128 experts, ratio 0.375 -> fired
Mixtral-8x7B 8 experts, ratio 3.500 -> did not
Found from a live failure. budsim reported 49.08 GB of weights for a Qwen3.5-35B-A3B
checkpoint whose own safetensors index says 71.90 GB; budcluster sized a 59 GiB pod
from that and the deployment OOMKilled loading weights. The heuristic was dropping
10.70B parameters -- 21.39 GB at bf16 -- which is the whole gap.
Sharing now requires an explicit key from `shared_down_keys`. No inference.
Also adds `qwen3_5_moe`, `qwen3_5_moe_text`, `qwen3_next` and `qwen3_5` to
`gated_ffn_model_types`. Without them a 3-matrix expert is charged as 2 -- a further
33% under-count -- resting entirely on `hidden_act` being present, which is the
unreliable signal that set exists to bypass.
Results:
Qwen3.5-35B-A3B 24.03B -> 34.73B params; weights -31.7% -> -3.4% vs the index
Qwen3-235B-A22B recovers 75B params; 234.5B vs 235.0B published (-0.2%)
Mixtral-8x7B unchanged; 46.6B vs 46.7B published (-0.3%)
dense models unchanged (no MoE term)
The residual -3.4% on Qwen3.5 is an under-counted vision tower (0.19B counted
against ~0.41B) plus ~1B of smaller terms, and it is still the dangerous direction
for a pod budget. Weights are exactly knowable from
`model.safetensors.index.json:metadata.total_size` when the checkpoint is mounted,
which would retire the estimate for this term entirely -- worth doing separately.
Testing:
pytest tests/test_moe_parameter_count.py -q # 8 passed
pytest tests/test_accuracy_remediation.py tests/test_kv_cache_geometry.py \
tests/test_lora_prefill_scratch.py -q # passed, no regressions
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…allback
Determining a model's weight memory has two methods, and counting parameters from
config.json is the SECOND one. If the checkpoint is readable its files are the answer,
and they cannot be wrong about an architecture they do not model.
That distinction is not theoretical. A heuristic here assumed 256 experts shared a
down-projection and under-counted Qwen3.5-35B-A3B by 47% -- 49.08 GB for a checkpoint
whose own index declares 71.90 GB -- and the pod sized from it OOMKilled loading
weights. After fixing that the estimate is still ~3.4% low, which is the direction
that kills pods.
`weights_from_checkpoint` tries, in order:
1. `*.index.json` -> `metadata.total_size` (authoritative, free)
2. sum of `*.safetensors` (single-file or sharded, no index)
3. sum of `*.bin` / `*.pth` / `*.pt` (pre-safetensors checkpoints)
`calculate_memory` applies it whenever it is handed a path, overriding the estimate
and recording which method answered in `MemoryReport.weight_source`. A caller sizing
a pod should know whether it got a measurement or an estimate, because only one of
them carries architecture risk.
It is a FALLBACK CHAIN, not a replacement. Of five real checkpoints in one registry:
one had a shard index, two a bare `model.safetensors`, one `pytorch_model.bin`, and
one no weight files at all -- registries commonly hold config and tokenizer while the
weights arrive later. The estimator stays load-bearing, which is why the MoE fix
mattered regardless.
Architecture-agnostic by construction: it branches on file layout, never on
`model_type`. Dense, MoE, hybrid linear-attention, multimodal and quantized exports
all answer the same way -- and a quantized export is the sharpest case, with the same
parameter count and a quarter of the bytes.
An index declaring more than the shards present is refused rather than measured. A
registry held exactly that: a 23 MB stub with an index claiming 66.97 GiB. Measuring
there reports ~0 and sizes a pod into an OOMKill carrying the authority of a
measurement.
Kept stdlib-only so a control plane can import it without the plotting stack.
This first landed in budcluster, because that is where the checkpoint happens to be on
disk at deploy time. That placed the logic by who holds the input rather than by what
owns the method -- the same error that had the LoRA scratch duplicated there -- so it
moves here, with its tests.
Testing: pytest tests/test_checkpoint_weights.py tests/test_moe_parameter_count.py \
tests/test_lora_prefill_scratch.py tests/test_kv_cache_geometry.py -q # 58 passed
Verified end to end on the real 35B: 71.90 GB measured via metadata.total_size,
recovering the 2.29 GiB the estimator misses; config-only falls back and says so.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…arrive weights_from_checkpoint refused to answer unless the shards were on disk, so the one caller that most needed the true number never got it. A control plane sizes a pod from the model registry -- a metadata cache holding config.json and the shard index -- and only then transfers the 67 GiB. Every real sizing call therefore saw an "incomplete" checkpoint and fell back to the estimator, which under-counts Qwen3.5-35B-A3B by 2.29 GiB. The pod was rendered at 78.96 GiB and OOMKilled with about that much paper headroom. The guard conflated two questions. "How many weight bytes are on disk" needs the shards. "How much weight memory will this model need" does not: metadata.total_size is a property of the model, equally true before, during and after the transfer. So the completeness check moves to the only path where a partial download can actually understate the answer -- summing files. A declared total does not shrink when shards are missing; a sum does. Summing now refuses when the -of-NNNNN in the filenames shows shards absent, which is the case that previously had no check at all. Verified against the live bud-dev registry: the 35B goes None -> 66.97 GiB, and the three other real checkpoints (bare safetensors, pytorch_model.bin, config-only) are byte-identical to before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Scope
Serving a LoRA-enabled model allocates two very different things, and this library reported
only the small one.
calculate_lora_adapter_memoryreturns the adapter A/B storage — 0.22 GiB at rank 256 ona 0.6B model. Punica's prefill working buffers at the same rank are 16.77 GiB, 75x larger,
and were reported as nothing at all. A control plane sizing a pod off the first number
under-sizes it by that factor; the pod then OOMKills at
Warming up model for the compilation,which destroys the evidence along with the pod.
This adds
ModelMemoryCalculator.calculate_lora_prefill_scratchand reports it as its ownMemoryReportfield. It is deliberately not folded into the storage term: summed together itreads as a rounding error on a number readers already believe is small, which is how a 16 GiB
allocation stayed invisible.
Calibration
Qwen3-0.6B, 28 layers, rank 256,
max_num_seqs=1, vLLM CPU backend, cgroup limit 40 GiB sonothing was clipped. LoRA-attributable is the LoRA-on peak minus a LoRA-off control pod at the
identical token budget:
Two results worth stating:
pod's context sensitivity is this one term.
max_num_batched_tokens, the tokens in one forward pass, not by thecontext length. With chunked prefill those differ, and charging the context prices an
8192-context pod at 31 GiB it never allocates.
API
MemoryReportgainslora_prefill_scratch_bytesandnotes;total_memory_bytesincludesthe scratch.
calculate_memory(...)gainsmax_num_batched_tokens, defaulting toseq_length. Callersthat do not set it get the unchunked reading, so nothing existing changes shape.
Where it stops, and says so
Above 4273 batched tokens the term goes convex — at 8192 the linear form under-predicts a
measured floor by >13 GiB — so it returns a note marking the figure a LOWER BOUND rather
than an estimate. It also notes when a request leaves the measured 28-layer/rank-256 geometry,
since linearity in depth and in rank is assumed rather than measured.
Testing
14 passed. New file
tests/test_lora_prefill_scratch.pypins both calibration points, thebatch-not-context behaviour, rank and TP scaling, both notes, and the refusal paths (unknown
layer count, unset rank).
Full suite: 8 pre-existing failures, all missing packages in the local environment
(
pymoox3;llm_memory_calculatornot pip-installed, which breaks the 5test_bud_componenttests that spawn a subprocess). None relate to this change.
Downstream
BudCluster consumes this calibration for pod sizing in BudEcosystem/bud-runtime#2422. It
restates the constants rather than importing the library — it is a control plane, and this
package pulls matplotlib/plotly/seaborn/pandas — and pins the two copies together with a test.
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.