Skip to content

fix(cache): correct mask offset for batched quantized KV caches - #76

Merged
inureyes merged 1 commit into
mainfrom
fix/75-batch-quant-mask-offset
May 24, 2026
Merged

inureyes merged 1 commit into
mainfrom
fix/75-batch-quant-mask-offset

Conversation

@inureyes

Copy link
Copy Markdown
Member

Closes #75

Batched quantized KV caches built an incorrect causal mask for left-padded sequences and lacked a dedicated mask-construction method. Mirrors the upstream mlx-vlm fix in PR #1208 (commit 2a55b80).

Root cause

The mask offset must come from the number of tokens actually stored in the buffer. The cache offset field starts at -left_padding[i] and is negative for padded sequences, so using it as the mask offset produced incorrect causal mask shapes and wrong attention masking during batched quantized decode.

Fix

  • Track the buffered-token count (idx) separately from offset in both BatchQuantizedKVCache and BatchTurboQuantKVCache, advancing it in update_after_decode alongside offset. The filter and extend paths trim and combine idx consistently with per-sequence left padding.
  • Add make_mask(n) to both caches using idx (not offset) as the mask offset and left_padding to exclude per-sequence padding positions. The single-token, no-padding case returns no mask.
  • Add create_causal_mask_with_left_padding to utils.rs, a causal-mask builder that accepts a left_padding argument.

Verification

  • cargo fmt --check clean.
  • cargo clippy --features metal,accelerate --all-targets -- -D warnings clean.
  • 33 unit tests pass, covering the buffered-token advance, the idx-vs-offset mask offset, left-padding exclusion, and the single-token no-mask path.

Batched quantized KV caches built an incorrect causal mask for left-padded sequences and lacked a dedicated mask-construction method. Mirrors the upstream mlx-vlm fix in PR #1208 (commit 2a55b80).

The mask offset must come from the number of tokens actually stored in the buffer. The cache `offset` field starts at `-left_padding[i]` and is negative for padded sequences, so using it as the mask offset produced incorrect causal mask shapes and wrong attention masking during batched quantized decode.

Changes:

- Track the buffered-token count (`idx`) separately from `offset` in both `BatchQuantizedKVCache` and `BatchTurboQuantKVCache`, advancing it in `update_after_decode` alongside `offset`. The `filter` and `extend` paths trim and combine `idx` consistently with per-sequence left padding.
- Add `make_mask(n)` to both caches using `idx` (not `offset`) as the mask offset and `left_padding` to exclude per-sequence padding positions. The single-token, no-padding case returns no mask.
- Add `create_causal_mask_with_left_padding` to `utils.rs`, a causal-mask builder that accepts a `left_padding` argument so padded positions are masked out per sequence.

Verified with cargo fmt, cargo clippy (metal,accelerate, all-targets, -D warnings), and 33 unit tests covering the buffered-token advance, the idx-vs-offset mask offset, left-padding exclusion, and the single-token no-mask path.
@inureyes inureyes added type:bug Bug fixes, error corrections, or issue resolutions priority:medium Medium priority area:core mlxcel-core: MLX FFI, primitives, KV cache, layers labels May 24, 2026
@inureyes
inureyes merged commit 9c814ec into main May 24, 2026
4 checks passed
@inureyes
inureyes deleted the fix/75-batch-quant-mask-offset branch May 24, 2026 14:09
@inureyes inureyes self-assigned this May 25, 2026
inureyes added a commit that referenced this pull request May 26, 2026
Bump version to 0.0.31. Highlights: MiniCPM-V 4.6 and RT-DETRv2 model support plus an Anthropic-style /v1/messages API endpoint (#82, #80, #74); fixes for missing or null chat-message content, Gemma 3n VLM per_layer_inputs, Qwen3.5 MTP verify attention, and batched quantized KV cache mask offsets (#91, #86, #78, #76); and a pinned 1.93.1 Rust toolchain (#87, #90).
@inureyes inureyes added this to the 0.1 milestone Jun 21, 2026
@inureyes inureyes added the status:done Completed label Sep 6, 2026
be-free-snowboy pushed a commit to freestylejs/snowbrain-mlxcel that referenced this pull request Oct 27, 2026
…up#76)

Batched quantized KV caches built an incorrect causal mask for left-padded sequences and lacked a dedicated mask-construction method. Mirrors the upstream mlx-vlm fix in PR lablup#1208 (commit 2a55b80).

The mask offset must come from the number of tokens actually stored in the buffer. The cache `offset` field starts at `-left_padding[i]` and is negative for padded sequences, so using it as the mask offset produced incorrect causal mask shapes and wrong attention masking during batched quantized decode.

Changes:

- Track the buffered-token count (`idx`) separately from `offset` in both `BatchQuantizedKVCache` and `BatchTurboQuantKVCache`, advancing it in `update_after_decode` alongside `offset`. The `filter` and `extend` paths trim and combine `idx` consistently with per-sequence left padding.
- Add `make_mask(n)` to both caches using `idx` (not `offset`) as the mask offset and `left_padding` to exclude per-sequence padding positions. The single-token, no-padding case returns no mask.
- Add `create_causal_mask_with_left_padding` to `utils.rs`, a causal-mask builder that accepts a `left_padding` argument so padded positions are masked out per sequence.

Verified with cargo fmt, cargo clippy (metal,accelerate, all-targets, -D warnings), and 33 unit tests covering the buffered-token advance, the idx-vs-offset mask offset, left-padding exclusion, and the single-token no-mask path.
be-free-snowboy pushed a commit to freestylejs/snowbrain-mlxcel that referenced this pull request Oct 27, 2026
Bump version to 0.0.31. Highlights: MiniCPM-V 4.6 and RT-DETRv2 model support plus an Anthropic-style /v1/messages API endpoint (lablup#82, lablup#80, lablup#74); fixes for missing or null chat-message content, Gemma 3n VLM per_layer_inputs, Qwen3.5 MTP verify attention, and batched quantized KV cache mask offsets (lablup#91, lablup#86, lablup#78, lablup#76); and a pinned 1.93.1 Rust toolchain (lablup#87, lablup#90).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core mlxcel-core: MLX FFI, primitives, KV cache, layers priority:medium Medium priority status:done Completed type:bug Bug fixes, error corrections, or issue resolutions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(cache): correct mask offset for batched quantized KV caches

1 participant