Repository navigation
Conversation
cache_config is documented as configuring the KV cache, but on the static
path it was read only inside the quantized branch, so an explicit
cache_config={"max_cache_len": N} was ignored and the cache was sized to
max_length-1. Thread it into _prepare_static_cache, guarded with max() so the
cache is never under-sized. No behavior change when cache_config is unset.
Adds a regression test in the no-download CacheTest class.
Fixes #46424
|
View the CircleCI Test Summary for this PR: https://huggingface.co/spaces/transformers-community/circle-ci-viz?pr=46700&sha=b2fb78 |
|
CI Dashboard: View test results in Grafana |
|
Ready for review. The single failing check (
This diff only touches the static-cache path in |
|
Immediate anti-spam block, sorry! There was a PR open at #46446 for the same issue which your bot didn't bother to check, then it pinged two core maintainers to ensure it wasted the maximum amount of everyone's time. |
|
@Rocketknight1 ah, I thought I checked but I clearly missed #46446 In my defense I took care to reproduce the bug and include runtime traces Thanks anways |
Fixes #46424 x
What
GenerationConfig.cache_configis documented as carrying "arguments used in the key-value cache class", but on the static-cache path it is silently ignored. Settingcache_config={"max_cache_len": N}together withcache_implementation="static"produces a cache sized tomax_length - 1, notN:The practical cost (from the issue): a second
generate()with a largermax_new_tokenspays a fullStaticCacherealloc +torch.compilerecompile, because the auto-allocated cache was sized for the first call. The documented way to avoid that — pin the size viacache_config— is exactly the path that doesn't work today.Root cause
In
GenerationMixin._prepare_cache_for_generation(src/transformers/generation/utils.py),cache_configis read only inside theelif generation_config.cache_implementation == "quantized"branch. The static branch sizes the cache straight from themax_cache_lengthargument (= generation_config.max_length - 1, computed by the caller) and never consultscache_config:A runtime trace of the repro confirms it:
max_cache_lengtharrives as47, the static branch is taken, and neither the quantized-branch condition nor thecache_configread ever executes (trace below).When it was introduced (not a regression)
git bisecthas no "good" commit here — the static path has never honoredcache_config.cache_configwas introduced as a QuantizedCache config object, and every refactor since has kept it quantized-only:At both #43679 and its parent commit,
cache_configis read only on the quantized branch. So this is a long-standing gap dating to the field's origin (#30483), not a regression.Fix
Thread
cache_config["max_cache_len"]into the static path, guarded withmax()so the cache is never under-sized for the current call:When
cache_configisNoneor carries nomax_cache_len, the value is unchanged — no behavior change for the default path.max(...)means an explicit-but-too-smallmax_cache_lencan never shrink the cache below what the current call needs.Tests
Added
tests/utils/test_cache_utils.py::CacheTest::test_static_cache_honors_cache_config_max_cache_len(in the existing no-download / no-GPUCacheTestclass): builds a tiny random Llama, runsgenerate()withcache_implementation="static"andcache_config={"max_cache_len": 2048}, and asserts the allocated cache is sized to2048.AssertionError: 47 != 2048(the bug).Runtime trace — before the fix (the diagnosis)
Captured by instrumenting
src/transformers/generation/utils.pyand running the repro.file:lineare pristine source coordinates.The complete set of events between lines 1920–1960 is the four shown above. There is no event at line 1938 (
elif … == "quantized") and no event at line 1945 (cache_config = …): the only code that readscache_configis never reached on the static path, so the requested2048is structurally unreachable and47sizes the cache.Runtime trace — after the fix (verification)
Who can help
@ArthurZucker @Cyrilvallez