Skip to content

Fix DynamicCache.reset() leaving the cache in a corrupt, non-empty state - #48509

Open
tritsystem wants to merge 1 commit into
huggingface:mainfrom
tritsystem:fix-dynamiccache-reset
Open

Fix DynamicCache.reset() leaving the cache in a corrupt, non-empty state#48509
tritsystem wants to merge 1 commit into
huggingface:mainfrom
tritsystem:fix-dynamiccache-reset

Conversation

@tritsystem

@tritsystem tritsystem commented Sep 3, 2026

Copy link
Copy Markdown

CPU CI GPU run-slow

Fix DynamicCache.reset() leaving the cache in a corrupt, non-empty state

What does this PR do?

Cache.reset() is a public method. On a DynamicCache (the default cache for
generative models) it does not empty the cache — it leaves it in a corrupt
state where get_seq_length() still reports the old length and the next
update() concatenates onto stale zeros.

Reproduction (on main)

import torch
from transformers import DynamicCache

c = DynamicCache()
c.update(torch.randn(1, 4, 6, 8), torch.randn(1, 4, 6, 8), layer_idx=0)
print(c.get_seq_length(), c.is_initialized)   # 6 True

c.reset()
print(c.get_seq_length(), c.is_initialized)   # 6 True    <-- expected 0, False

k, _ = c.update(torch.randn(1, 4, 3, 8), torch.randn(1, 4, 3, 8), layer_idx=0)
print(k.shape[-2])                             # 9         <-- expected 3
# k[..., :6, :] is all-zero stale state

Root cause

CacheLayerMixin.reset() zeroes keys / values in place (shape unchanged)
and leaves is_initialized = True:

def reset(self) -> None:
    if self.is_initialized:
        self.keys.zero_()
        self.values.zero_()
    if hasattr(self, "cumulative_length"):
        ...

That is correct for StaticLayer — a pre-allocated buffer whose logical length
is a separate cumulative_length counter. DynamicLayer has no length
counter: its length is keys.shape[-2], and it grows via torch.cat. It does
not override reset(), so after reset() the tensors are zeroed but keep their
old shape and is_initialized stays Trueget_seq_length() returns the
stale length and update() does torch.cat([stale_zeros, new_states], dim=-2).

Same for DynamicSlidingWindowLayer (cumulative_length gets reset by the base,
but the K/V tensors and is_initialized do not) and DynamicIndexedLayer (its
reset() calls super().reset() and then zeroes the also-concatenation-grown
indexer_keys in place).

generate() itself is unaffected — it only ever resets a StaticCache — but any
user code that reuses a cache across calls hits this.

Fix

  • Add DynamicLayer.reset() that returns the layer to its uninitialised
    state (keys = values = None, is_initialized = False, and
    cumulative_length = 0 when present, covering DynamicSlidingWindowLayer).
  • DynamicIndexedLayer.reset(): empty indexer_keys (None +
    is_indexer_initialized = False) instead of zeroing it in place.
  • LinearAttentionAndFullAttentionLayer.reset() /
    LinearAttentionAndSlidingWindowAttentionLayer.reset() already delegate to
    DynamicLayer.reset / DynamicSlidingWindowLayer.reset, so they are fixed
    transitively.

After the fix, DynamicCache.reset() gives get_seq_length() == 0,
is_initialized is False, and a subsequent update(3) returns a length-3
sequence with no stale positions. reset() on a fresh cache stays a no-op.

Not in scope (noted)

QuantizedLayer (KIVI-style quantized KV cache) subclasses DynamicLayer and
gets the keys / values / is_initialized reset from this change, but keeps
its _quantized_keys / _quantized_values residual buffers. If you'd like, I can
add a small QuantizedLayer.reset() in this PR or a follow-up.

Tests

tests/utils/test_cache_utils.py::SyntheticCacheTest::test_dynamic_cache_reset_returns_to_empty_state
— fails on main (AssertionError: 6 != 0), passes with this change.

pytest tests/utils/test_cache_utils.py -q -k "SyntheticCacheTest or CacheTest or Cropping"
# 17 passed

ruff check / ruff format --check clean on both files.

Before submitting

  • Read the contributor guideline.
  • This is a bug fix with a repro and a regression test.
  • Did you write any new necessary tests? — yes, one.

`CacheLayerMixin.reset()` zeroes `keys` / `values` in place with the shape
unchanged and leaves `is_initialized` True. That is correct for `StaticLayer`
(a pre-allocated buffer with a separate `cumulative_length` counter), but
`DynamicLayer` -- the layer inside `DynamicCache`, the default cache for
generative models -- has no length counter: its length *is* `keys.shape[-2]`,
and it grows by `torch.cat`.

So after `DynamicCache.reset()`:
  - `get_seq_length()` still returns the old length,
  - `is_initialized` stays True,
  - the next `update()` concatenates the new states onto the stale zeros,
returning `old + new` positions with `old` leading zero vectors.

Add `DynamicLayer.reset()` that returns the layer to its uninitialised state
(`keys = values = None`, `is_initialized = False`, and `cumulative_length = 0`
for `DynamicSlidingWindowLayer`). Update `DynamicIndexedLayer.reset()` to empty
the (also concatenation-grown) indexer key cache rather than zero it in place.
`LinearAttentionAndFullAttentionLayer` / `...SlidingWindowAttentionLayer`
already delegate to these, so they are fixed too.

Adds `SyntheticCacheTest.test_dynamic_cache_reset_returns_to_empty_state`
(fails on main, passes with this change).
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

CI recap

Dashboard: View test results in Grafana
Latest run: 33769974777
Result: success | Grafana metrics are not available yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant