Repository navigation
Fix several bugs in cache_implementation=static - #46446
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes several correctness and stability issues when using cache_implementation="static" during generation, especially for chunked prefill and hybrid models that include linear-attention layers.
Changes:
- Make static-cache sizing respect
generation_config.cache_config["max_cache_len"]to avoid reallocations/recompiles across calls. - Fix static-cache eager initialization and cache metadata (
is_initialized,max_cache_len) to safely handle hybrid linear-attention layers. - Make mask pre-building (
create_masks_for_generate) defer to the model when a layer type has no registered mask builder, avoidingKeyErrorfor hybrids.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/transformers/generation/utils.py |
Adjusts static-cache sizing and adds eager cache initialization for chunked prefill. |
src/transformers/cache_utils.py |
Makes early_initialization, is_initialized, and max_cache_len robust to linear-attention layers; updates docs. |
src/transformers/masking_utils.py |
Defers mask pre-building when encountering unmapped layer_types. |
tests/utils/test_cache_utils.py |
Adds regression tests for eager-init behavior, hybrid linear-attention handling, and max_cache_len honoring. |
tests/utils/test_masking_utils.py |
Adds regression test ensuring unmapped layer types defer mask creation instead of raising. |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
With `prefill_chunk_size` set, `generate()` compiles the prefill (via `get_compiled_call()` per chunk). It hits a condition in cache.update that leads to a graph specialization between the first call to generate and the second (one has the cache init, the second doesn't), hence a recompilation. The fix is to force a call to early_initialization outside of the compile region when chunked prefill. Also corrects the `lazy_initialization` docstring, which claimed `early_initialization` was unnecessary for `compile` because the prefill is never compiled internally — not true once `prefill_chunk_size` is set. Fixes #46421 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`GenerationConfig.cache_config` is documented as carrying arguments for the
key-value cache, but `_prepare_cache_for_generation` read it only on the
quantized branch. The static branch hardcoded `max_cache_len = max_length - 1`,
so `cache_config={"max_cache_len": N}` with `cache_implementation="static"` was
silently ignored.
Fixes #46424
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`early_initialization` initialized every layer via the `(num_heads, head_dim)` key/value layout, but linear attention layers (mamba/conv/linear_attention) don't use that layout — they track their own statically-shaped conv/recurrent states, initialized lazily on update. Initializing them through the key/value path is wrong and crashes on the first call to prefill. Fixes #46439 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`generate()` pre-builds attention masks for compilable caches via helpers registered in `LAYER_PATTERN_TO_MASK_FUNCTION_MAPPING`. No helpers exist for hybrid linear-attention models (Qwen3.5, Qwen3-Next, ...), since they build their own masks in the forward, so this raised a cryptic `KeyError: 'linear_attention'` and made `cache_implementation="static"` unusable with these models. This skips the early creation of the mask for these models. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The `max_cache_len` property iterated every layer, but linear attention layers have no `max_cache_len`, so the auto-allocated static-cache reuse check in `_prepare_static_cache` raised `AttributeError` on a hybrid model. Skip them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ti-device device_map
915ade8 to
aa28be8
Compare
| def is_initialized(self) -> bool: | ||
| """Return whether the cache data is initialized""" | ||
| return len(self.layers) > 0 and all(layer.is_initialized for layer in self.layers) | ||
| # Layers without an `is_initialized` flag (e.g. linear attention layers, which track their own conv/recurrent |
There was a problem hiding this comment.
i think it might be better to have them set that flag in general no?
There was a problem hiding this comment.
That's one option, yes, but this means we need to set the flag to True in LinearAttentionCacheLayerMixin, which I find a bit disturbing, because it's not really initialized.
Another option would be to have a class constant like supports_early_init.
The two checks become more readable:
if not supports_early_init or is_initialized:
continueAnd
return len(self.layers) > 0 and all(layer.is_initialized for layer in self.layers if layers.supports_early_init)There was a problem hiding this comment.
Implemented what i had suggested.
…nstead of getattr
|
CI Dashboard: View test results in Grafana |
…ked-prefill-recompile
Cyrilvallez
left a comment
There was a problem hiding this comment.
Nice, thanks for the PR! It indeed touches a few parts that we let a bit on the side until now (chunked prefill, linear layer early init).
IMO, the easiest and most foolproof way to proceed though would be to avoid compiling the first chunk of the prefill altogether, so that the cache will properly be early initialized as usual. This would allow early init even on multi gpu setup, and avoid checking dims in case of TP etc, which can be a hassle. I don't think it would harm performances in general. WDYT?
| # A user-provided `cache_config["max_cache_len"]` lets the static cache be sized for the worst case | ||
| # across calls, so that later calls with a longer prompt or a larger `max_new_tokens` (up to that | ||
| # ceiling) reuse the same cache instead of triggering a reallocation (and a `torch.compile` | ||
| # recompilation). Without this, the cache is sized to the current call's `max_length` only. See #46424. | ||
| cache_config = generation_config.cache_config or {} | ||
| if cache_config.get("max_cache_len") is not None: | ||
| max_cache_length = max(max_cache_length, cache_config["max_cache_len"]) | ||
| cache_batch_size = max(generation_config.num_beams, generation_config.num_return_sequences) * batch_size |
There was a problem hiding this comment.
Indeed, nice to have a mechanism to preset the length manually. I think it was not done before because cache_config is not really documented/well known, and I believe it was previously introduced mostly for quantized cache.
Maybe we could get this argument out of cache_config, wdyt? I believe it would probably make a bit more sense to have a properly documented max_cache_len directly? That way we can properly explain that it's used only with static cache, and that it avoids recompilation if several forward will be run with different lengths?
There was a problem hiding this comment.
OK, reverted the change and added a max_cache_len GenerationConfig parameter directly.
| elif prefill_chunk_size is not None: | ||
| # Chunked prefill compiles the prefill, so eagerly init the fresh cache to avoid a recompile next call | ||
| # (#46421). Skipped (-> lazy init) when it can't be initialized on a single device. | ||
| init_shape = self._get_static_cache_init_shape() | ||
| if init_shape is not None: | ||
| num_heads, head_dim = init_shape | ||
| self._cache.early_initialization( | ||
| batch_size=batch_size, | ||
| num_heads=num_heads, | ||
| head_dim=head_dim, | ||
| dtype=self.dtype, | ||
| device=self.device, | ||
| ) |
There was a problem hiding this comment.
Here, looks like we actually miss most of the cases, as it will only work for 1 gpu setup. The better way to proceed I believe would be to simply force the 1st chunk to NOT use compile, so that the cache is properly initialized before compile. WDYT? I don't think it would harm performance at all in general to have a single forward not compiled
There was a problem hiding this comment.
Let's take a step back. The whole point of using chunked prefill and static cache in combination is to have fully static graphs for prefill and decode, regardless of the prompt length and max new tokens. This might look like a mild optimization for CUDA devices (would need proper benchmarks to confirm), but it is essential on devices that do not support dynamic shapes (Neuron and TPU), for which eager mode is just "fragmented compilation".
Currently, without this fix, the prefill is compiled twice on any configuration.
The code I submitted fixes it for the configurations where it matters most. For the others (large model not using TP but sharded on multiple devices, incompatible head counts), it keeps the existing behaviour.
Your suggestion would fix those edge cases, but by never compiling the first prefill chunk on any configuration, which is very harmful for Neuron and TPU.
This reverts commit 90b0b61.
Expose a documented top-level `GenerationConfig.max_cache_len` to pre-size the static cache for the worst case across calls, so later calls with a longer prompt or larger `max_new_tokens` (up to that ceiling) reuse the same cache instead of reallocating and recompiling. Replaces the previously silently-ignored `cache_config["max_cache_len"]` path with an explicit, validated flag. Fixes #46424 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ked-prefill-recompile
…ked-prefill-recompile
…ked-prefill-recompile
|
View the CircleCI Test Summary for this PR: https://huggingface.co/spaces/transformers-community/circle-ci-viz?pr=46446&sha=6f39ad |
ArthurZucker
left a comment
There was a problem hiding this comment.
lgtm, not sure it will generalize well (_get_static_cache_init_shape)
| max_cache_len (`int`, *optional*): | ||
| Only used with static caches (`cache_implementation` set to `"static"` or `"offloaded_static"`). | ||
| Pre-sizes the cache to this length instead of the current call's `max_length`. Set it once to the | ||
| largest call you expect so that repeated `generate()` calls with a longer prompt or a larger | ||
| `max_new_tokens` (up to this ceiling) reuse the same cache instead of triggering a reallocation and a | ||
| `torch.compile` recompilation. |
There was a problem hiding this comment.
i am surprise this is not in generation config
| num_key_value_heads = getattr(text_config, "num_key_value_heads", None) or text_config.num_attention_heads | ||
| if num_key_value_heads % tp_size != 0: | ||
| return None | ||
| head_dim = getattr(text_config, "head_dim", None) or text_config.hidden_size // text_config.num_attention_heads | ||
| return num_key_value_heads // tp_size, head_dim |
There was a problem hiding this comment.
this does not necessarily scale well with DSv4 for example and qk_head_dim
There was a problem hiding this comment.
Added yet another exclusion
* Fix static-cache recompilation with chunked prefill With `prefill_chunk_size` set, `generate()` compiles the prefill (via `get_compiled_call()` per chunk). It hits a condition in cache.update that leads to a graph specialization between the first call to generate and the second (one has the cache init, the second doesn't), hence a recompilation. The fix is to force a call to early_initialization outside of the compile region when chunked prefill. Also corrects the `lazy_initialization` docstring, which claimed `early_initialization` was unnecessary for `compile` because the prefill is never compiled internally — not true once `prefill_chunk_size` is set. Fixes huggingface#46421 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Honor cache_config max_cache_len on the static cache path `GenerationConfig.cache_config` is documented as carrying arguments for the key-value cache, but `_prepare_cache_for_generation` read it only on the quantized branch. The static branch hardcoded `max_cache_len = max_length - 1`, so `cache_config={"max_cache_len": N}` with `cache_implementation="static"` was silently ignored. Fixes huggingface#46424 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Make StaticCache.early_initialization robust to linear attention layers `early_initialization` initialized every layer via the `(num_heads, head_dim)` key/value layout, but linear attention layers (mamba/conv/linear_attention) don't use that layout — they track their own statically-shaped conv/recurrent states, initialized lazily on update. Initializing them through the key/value path is wrong and crashes on the first call to prefill. Fixes huggingface#46439 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Let create_masks_for_generate defer for models with custom layer masks `generate()` pre-builds attention masks for compilable caches via helpers registered in `LAYER_PATTERN_TO_MASK_FUNCTION_MAPPING`. No helpers exist for hybrid linear-attention models (Qwen3.5, Qwen3-Next, ...), since they build their own masks in the forward, so this raised a cryptic `KeyError: 'linear_attention'` and made `cache_implementation="static"` unusable with these models. This skips the early creation of the mask for these models. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Make StaticCache.max_cache_len robust to linear attention layers The `max_cache_len` property iterated every layer, but linear attention layers have no `max_cache_len`, so the auto-allocated static-cache reuse check in `_prepare_static_cache` raised `AttributeError` on a hybrid model. Skip them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * review(generate): skip/shard eager static-cache init under TP and multi-device device_map * fix: typo * review(generate): move eager static-cache init into _prepare_static_cache * review(generate): isolate eager static-cache init args in a helper * review(cache): gate early-init via a supports_early_init layer flag instead of getattr * Revert "Honor cache_config max_cache_len on the static cache path" This reverts commit 90b0b61. * feat(generate): add max_cache_len generation flag for static caches Expose a documented top-level `GenerationConfig.max_cache_len` to pre-size the static cache for the worst case across calls, so later calls with a longer prompt or larger `max_new_tokens` (up to that ceiling) reuse the same cache instead of reallocating and recompiling. Replaces the previously silently-ignored `cache_config["max_cache_len"]` path with an explicit, validated flag. Fixes huggingface#46424 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * review: MLA models cannot be lazy initialized --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
What does this PR do?
Static-cache fixes for generate(), one commit each:
early_initializationcorrupts linear-attention layers (wrong shapes →RuntimeError) #46439).cache_implementation="static"—KeyError: 'linear_attention'#46441).Together these make static-cache + chunked-prefill generation work without recompiles for plain (Llama) and hybrid linear-attention (Qwen3.5) models. Each commit has a regression test.
Before submitting
Pull Request section?
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@ArthurZucker @Cyrilvallez @vasqu