Skip to content

feat: capture decode-step KV cache in auto-generated module configs - #136

Draft
kiszk wants to merge 2 commits into
mainfrom
feat/oot-cache-kwarg-reconstruction
Draft

feat: capture decode-step KV cache in auto-generated module configs#136
kiszk wants to merge 2 commits into
mainfrom
feat/oot-cache-kwarg-reconstruction

Conversation

@kiszk

@kiszk kiszk commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

The auto-generated module-test configs drive each DecoderLayer through both a
prefill and a decode invocation, but the forward hook was skipping
past_key_values entirely:

if key in ("past_key_values", "past_key_value"):
    continue  # Skip - not needed for module-level tests

As a result the decode invocation was replayed with past_key_values=None. In
HF attention the cache-update branch

if past_key_values is not None:
    key_states, value_states = past_key_values.update(key_states, value_states, self.layer_idx)

is then never taken, so the "decode" step silently degrades to a 1-token
self-attention (Q[1] × K[1]) instead of attending over the cached past
.
Both eager and compiled runs share the same wrong input, so test_eager_vs_compile
passes without actually exercising decode.

This PR captures the decode-step KV cache so the module test can rebuild an
equivalent cache and drive the real "attend over past + new token" path. The
companion torch-spyre change reconstructs the cache on the test side.

Changes (utils/module_discovery/auto_generate_module_config.py)

  • _resolve_layer_idx(module) — resolves the decoder layer index. In
    transformers ≥5 layer_idx lives on self_attn, not the DecoderLayer, so
    we fall back through both.
  • _extract_cache_info(...) — snapshots the target layer's populated K/V slice
    from a StaticCache into a cache spec (cache_path, layer_idx,
    key/value, max_cache_len, config_path, config_kwargs):
    • Uses the per-layer get_seq_length(), not the whole-cache length
      (which over-counts by one at layer i>0 once earlier layers have updated
      this pass).
    • Records only the populated [:, :, :past_len, :] slice (StaticCache
      allocates the full max_cache_len up front).
    • Only StaticCache is recorded; a growable DynamicCache has no fixed
      shape to reconstruct, so it emits a warning and is skipped.
    • Prefill (empty slot) records nothing — equivalent to past_key_values=None.
  • Hook no longer skips past_key_values; it records the cache spec instead.
  • _convert_captured_input_to_sample_input / _extract_pattern gain a cache
    branch (the latter keeps prefill and decode as distinct invocation signatures).
  • _validate_cache_mask_consistency(...) — generation-time sanity check that a
    cached decode invocation also carries an attention_mask whose key length can
    cover the cached past (logged, non-fatal).

Verification (CPU, transformers 5.12)

  • Ran the capture path on a tiny GptOssForCausalLM (same modeling code as
    gpt-oss-20b, incl. past_key_values.update()) driven with an explicit
    StaticCache; the written YAML's GptOssDecoderLayer decode invocation
    contains the cache spec (StaticCache, layer_idx=0, key
    [1, 2, 16, 16] = [B, num_kv_heads, past_len, head_dim]).
  • End-to-end: capture → reconstruct StaticCache (seq_length 16) → real
    DecoderLayer decode forward runs update() over past 16 + new 1 token
    (output [1, 1, 64], cache length 16 → 17).
  • DynamicCache still warns and records no cache.

Notes for reviewer

Signed-off-by: Kazuaki Ishizaki <ishizaki@jp.ibm.com>
@kiszk
kiszk force-pushed the feat/oot-cache-kwarg-reconstruction branch from ef4d93d to 22048dc Compare August 5, 2026 09:45
Signed-off-by: Kazuaki Ishizaki <ishizaki@jp.ibm.com>
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