add TurboQuant KV cache compression#232
Conversation
| ], | ||
| dependencies: [ | ||
| .package(url: "https://github.com/ml-explore/mlx-swift", .upToNextMinor(from: "0.31.3")), | ||
| .package(url: "https://github.com/ekryski/mlx-swift", branch: "alpha"), |
There was a problem hiding this comment.
You cannot change this library, I mean this needs to reference the official ml-explore mlx-swift, if you need to add there, you need to first add a pull request there and get it merged, this is no sense.
There was a problem hiding this comment.
This PR is still in draft, pointing to the wrong library. I would hold off on review. Defiant pointing at the wrong library
There was a problem hiding this comment.
This will be fixed in the next push. No intention of switching the dependencies.
There was a problem hiding this comment.
Out-of-bounds threadgroup memory in fused encode kernels (can corrupt results/crash for head_dim > 128).
In both dense and WHT encode kernels, shared_norm is hardcoded to 4 entries, but indexed by sg_id = d/32 and read up to num_groups = (Dim+31)/32. For Dim=256, this accesses indices 0...7.
There was a problem hiding this comment.
Good catch, fixed in next push. shared_norm is now sized [(Dim + 31) / 32] instead of hardcoded 4. Current models all use head_dim=128 so this wasn't hit in practice but would break on 256.
| } | ||
|
|
||
| // KV head mapping (use first query's head — same assumption as non-causal NR0) | ||
| uint q_head_idx_0 = (query_group * NR0) / L; |
There was a problem hiding this comment.
NR0 path derives kv_idx from only the first query row in the group. If a group spans multiple query heads (possible when grouping is not aligned to L), some rows will read/write against the wrong KV head. Guarding only on totalQ % nr0 == 0 is insufficient for correctness.
There was a problem hiding this comment.
Added a queryChunkLength % nr0 == 0 in, next push, guard so the NR0 path only activates when L aligns with the group size. Falls back to per-row dispatch otherwise.
There was a problem hiding this comment.
TurboQuant cache cannot be restored correctly from prompt cache. TurboQuantKVCache is not represented in cacheClassName, so it is serialized as KVCache. On load it is restored as KVCacheSimple, but TurboQuant compressed state carries 3/4 arrays rather than KVCacheSimple’s required 2 arrays, leading to invalid restoration behavior. Add explicit TurboQuant class name mapping and restore path.
There was a problem hiding this comment.
Fixed in nex push. Added TurboQuantKVCache to cacheClassName and restoreCacheFromMetaState. metaState now carries bits, keyBits, valueBits, and seed so the cache can be reconstructed correctly on load.
|
@aleroot I appreciate the early reviews but this is still in draft. |
| self.maxTokens = parameters.maxTokens | ||
| self.numDraftTokens = numDraftTokens | ||
|
|
||
| self.quantizeKVCache = { cache in |
There was a problem hiding this comment.
kvScheme dropped in speculative decoding quantization.TurboQuant scheme selection is ignored for speculative generation despite being part of GenerateParameters...
There was a problem hiding this comment.
Yep, missed that. Threaded kvScheme through the speculative quantization closure now.
aleroot
left a comment
There was a problem hiding this comment.
The implementation is ambitious and has strong ideas, and honestly is something I wanted to work on as well... At the moment this pull request is just a prototype as it does not look like a full inner-product TurboQuant formulation ...
There are no tests as well that are really proving is working.
Appreciate your early feedback. Still a work in progress and know that this is an ambitious PR as is. Thank you for the draft comments! |
Sorry, I did not notice it was a draft . |
All good let's work together on this. It's a big lift and it's hard to separate into individual PRs. Lots of work ahead. |
Follow up as i was on the road for my last comment: Still polishing this, the PPL data in the description shows it working across Llama, Qwen, Mistral, and Qwen3.5 MoE. Happy to collaborate on getting this to a state that works for the project. If you have ideas on the formulation or testing approach I'm all ears. (we can also do email or discord or whatever works) |
4921784 to
2864b83
Compare
d4ef67a to
61ca2a1
Compare
|
@TheTom what is the state of this one? Trying to dig out of the backlog, so no rush :-) |
|
Thanks for the ping, just been busy with other projects. will bump it up and report back soon |
61ca2a1 to
d246ec7
Compare
| /// validate on your model (asym is the recommended starting point). | ||
| /// | ||
| /// Unrecognized schemes are ignored. See `resolveTurboScheme`. | ||
| public var kvScheme: String? |
There was a problem hiding this comment.
I wonder if this should be an enum? I realize it is a little late (pre-existing) but potentially we could have an enum property and have this one be computed and set the other one.
There was a problem hiding this comment.
Ah, it is in parameters too -- perhaps that ship has sailed.
There was a problem hiding this comment.
I'm flexible just tell me what you prefer as the repo owner
There was a problem hiding this comment.
Let's leave it as-is. I would prefer enum, but I don't know if it is feasible and it is outside the scope of this PR. We can think about it in a followup. Maybe we only need the enum here and have the rest of the parameters stay as String -- I think it may be more important for callers to have an easy way to know what values are possible.
There was a problem hiding this comment.
Yeah agreed, enum would be nicer but the string is already in parameters so it's a bit baked in. Happy to do a follow-up that adds an enum for discoverability and keeps the string as the storage. For now I left it alone like you said.
| [UInt32(tokenCount), UInt32(repeatCount), UInt32(numBlocks), 1, 0] as [UInt32]) | ||
| let partials = kernel( | ||
| [ | ||
| f32(rotatedQueries), rawKeys.asType(.float16), |
There was a problem hiding this comment.
I am curious about the width conversion on rawKeys (which per the call to update() are in the dtype of the activation weights). float32 rawKeys are unlikely, but what about bfloat16? The exponent may be too large and convert to (I presume) an infinity. It looks like the math in the kernels are float32, so it is really only an issue for the inputs.
Is this a real issue? Does this require bfloat16 and float32 kernels (or at least bfloat16)?
Related: I think if we don't have bfloat16 kernels, this will repeatedly cast from bfloat16 to float16, potentially leading to n^2 cost.
There was a problem hiding this comment.
Good catch, and it was actually worse than the overflow. That asType was also copying the whole raw K cache every decode step since the cache grows, so the n^2 you suspected was real too.
Fixed by just not casting. mlx generates the kernel signature from the runtime dtype of whatever array you pass in, so I made the K pointer type a Dtype template arg (KT, plus KScaleT/KBiasT for the affine scales/biases which had the same problem). Template args get hashed into the kernel name so each dtype gets its own compiled variant, one JIT compile per dtype ever used. Had to do it that way btw, mlx's custom kernel library cache is single slot per kernel name, so without the dtype in the name alternating dtypes would recompile every switch.
So to answer your question directly: yes it needs bf16/f32 variants and now it has them for free. Added a test that runs the raw-K decode with a bf16 cache containing a key above 65504 and checks against an f32 reference, no inf.
d246ec7 to
c9aba7a
Compare
| keyBits: Int, | ||
| valueBits: Int, | ||
| quantizedKVStart: Int | ||
| ) { |
There was a problem hiding this comment.
I think this needs a groupSize check like maybeQuantizeKVCache() has:
func quantize(_ cache: KVCacheSimple) -> KVCache {
let state = cache.state
if state.count == 2 {
let keyHeadDim = state[0].dim(3)
let valueHeadDim = state[1].dim(3)
guard
resolvedKVQuantizationGroupSize(
requested: effectiveGroupSize,
keyHeadDim: keyHeadDim,
valueHeadDim: valueHeadDim
) != nil
else {
return cache
}
}
return cache.toQuantized(groupSize: effectiveGroupSize, bits: effectiveBits)
}Specifically resolvedKVQuantizationGroupSize() will return nil if the headDim are incompatible with the groupSize (affineKeyGroupSize) and it won't try to quantize the cache.
There was a problem hiding this comment.
QuantizedKVCache also has this:
public func updateQuantized(keys: MLXArray, values: MLXArray) -> (
(MLXArray, MLXArray, MLXArray?), (MLXArray, MLXArray, MLXArray?)
) {
...
guard effectiveGroupSize != nil else {
fatalError(
"KV cache quantization requires head dimensions divisible by one of the supported group sizes (32, 64, 128). Requested group size: \(groupSize). Key head dim: \(kHeadDim). Value head dim: \(vHeadDim)."
)
}I think you could only get to that case if you constructed it directly, but a similar message might be worthwhile -- it is a programmer error so a fatal is ok, but giving an actionable message is useful.
There was a problem hiding this comment.
Done, used resolvedKVQuantizationGroupSize exactly like maybeQuantizeKVCache does. Covers both the turbo8 schemes and the boundary-layer protection, skips the layer (stays fp16) when nothing fits, and if it resolves to something other than 64 (like 32 for head dim 96) that gets threaded through to the kernels too so they always agree. Tests at head dim 80 for both paths.
One thing I wasn't sure about: on an incompatible head dim the value side could technically still compress (the codebook path has no group size constraint), so turbo8v3 could degrade to turbo0v3 instead of skipping entirely. I went with skip to match the existing semantics but easy to flip if you'd rather degrade gracefully.
There was a problem hiding this comment.
Yep, added. The affine-K encode now resolves the group size up front and the fatal prints the head dim and the supported sizes, same style as updateQuantized. Only reachable by constructing the cache directly but at least now it tells you what to fix.
Walsh-Hadamard rotation + Lloyd-Max codebook KV compression behind the kvScheme parameter (ml-explore#230), with JIT Metal kernels: fused encode, a SIMD-parallel two-pass online-softmax flash decode (packed-K, raw-fp16-K, and 8-bit-affine-K score variants), and an inline sparse value kernel. Schemes are asymmetric because keys are far more quality-sensitive than values (softmax amplifies key error, value error averages out): the recommended tiers keep K at fp16 (turbo0v4/v3/v2) or 8-bit affine (turbo8v4/v3/v2) while V takes the compression; symmetric turbo4/3/2 compress both sides. Symmetric key quality rides on per-dimension key calibration: at compression time the cache derives a per-dimension scale from the post-rotation key variance of the prefill cache and folds it into the query side, leaving the score kernels untouched; the calibrated encode rotates through the codec's MLX ops so kernel and reference quantize identical values. Boundary-layer protection converts the first and last attention layers to 8-bit affine for fragile schemes. Rotating (sliding-window) cache layers stay fp16 with a one-time notice; global layers on mixed models compress normally. Per-call kernel values travel in a parameter buffer rather than template arguments, so each shape compiles once. Caches stay raw fp16 through prefill, compress on the first decode step, and encode incrementally; state round-trips through prompt cache save/restore, and speculative decoding routes through the compressed path. Generation settles cache state with the token eval and drains autoreleased temporaries per step so long generations hold steady memory.
Codec round trips and pinned codebooks, rotation orthogonality, packing, kernel-vs-reference attention parity (GQA repeat factors 1/2/3, non-power-of-two head dims), calibrated-encode kernel exact index parity, prompt-cache round trips for all state layouts including calibration state, compressed partial trim with continued decode, speculative decoding with a turbo scheme, allocation-boundary growth, boundary-protection gating, mixed rotating/standard cache conversion, and end-to-end generation on tiny random-weight models. CI-runnable, no checkpoint downloads.
Scheme reference table, the asymmetric-first selection ladder, measured family sensitivity and calibration notes, performance guidance, and limitations.
|
Pushed the review fixes: native bf16/f32 dtype variants for the raw-K and affine-K kernels (no more per-step casts), the group size guard with tests at head dim 80, and the actionable fatal. Also dropped some dead experimental kernels I found while in there. 63 tests now. CI will need an approve-and-run when you get a chance, it got gated after the force push. |
c9aba7a to
b64793c
Compare
davidkoski
left a comment
There was a problem hiding this comment.
Change looks good, thank you!
Proposed changes
Adds TurboQuant KV-cache compression (arXiv:2504.19874) behind the
kvSchemeparameter that landed in #230. This is a complete rework since the last push; the earlier draft's codec and kernels are gone.MLXFast.metalKernel(source-string JIT; no Package.swift or build changes): fused encode, and a SIMD-parallel two-pass online-softmax flash decode with three K-load variants (turbo-packed, raw fp16, 8-bit affine) plus an inline sparse value pass. Per-call values travel in a parameter buffer rather than kernel templates, so each shape compiles once.turbo0v4/v3/v2) or 8-bit affine (turbo8v4/v3/v2, scored via inline dequant) while V takes the compression. Symmetricturbo4/3/2are available for validated use.Usage
Measured quality and compression
M5 Max, WikiText-2 decode-time evaluation, ctx 4096. KLD is mean full-vocab KL divergence against an fp16 cache with identical weights, so it isolates cache compression from weight quantization. Compression is measured cache bytes against the fp16 cache.
Reading the table:
Decode throughput
Single-token decode uses one SIMD-parallel flash dispatch that reads K once in its stored basis (raw fp16 or inline-dequantized affine, no full-cache cast or GQA materialization), dequantizes V inline with a sparse gate, and applies the inverse value rotation in the reduction pass. Qwen3-1.7B, tok/s:
Family sensitivity notes
Tests
60 tests, CI-runnable, no checkpoint downloads: codec round trips and pinned codebooks, rotation orthogonality, packing, kernel-vs-reference attention parity (GQA repeat factors 1/2/3, non-power-of-two head dims), prompt-cache save/restore for all four state layouts, compressed partial trim with continued decode, speculative decoding with a turbo scheme, allocation-boundary growth, boundary-protection gating, end-to-end generation on a tiny random-weight model, calibrated-encode kernel-vs-reference exact index parity, calibration state save/restore, and mixed rotating/standard cache conversion.
Not in this PR (follow-ups)
Review history
All five earlier review items are resolved in this revision: Package.swift untouched, threadgroup sizing for head dims above 128, NR0 KV-head-boundary gating, prompt-cache restore, and speculative-decoding kvScheme plumbing.
Checklist
pre-commit run --all-filesto format my code<doc:kv-cache-quantization>article +GenerateParameters.kvScheme)