Skip to content

Commit a49e0f8

Browse files
titaiwangmsCopilot
andauthored
Fix nonpad_kv_seqlen in static-cache generation example (before-scatter -> after-scatter) (#365)
## What `examples/static_cache_generation.py` fed `nonpad_kv_seqlen` as the **before-scatter** KV count. A separate `nonpad_kv_seqlen` accumulator stayed always-equal to `write_indices` (both initialized to 0, both incremented only *after* `session.run`), so the example passed `nonpad == write_indices`. The spec-correct value is `write_indices + cur_seq_len` — the valid KV count **after** the current chunk is scattered into the cache, per the bottom-right `is_causal` contract (onnx/onnx#8068). ## Impact Wrong in **every** phase: - **Prefill**: passed `0` instead of `prompt_len`, so the kernel saw ~0 valid keys and produced wrong logits. - **Decode**: off-by-one on every step. On the CPU EP the maskless graph **runs-but-wrong silently** (no error raised). This is **example-only** — it is the canonical usage reference and does **not** affect library/graph code or PR #364. ## Fix - Feed `nonpad_kv_seqlen = write_indices + cur_seq_len` at the run inputs. - Remove the now-dead redundant `nonpad_kv_seqlen` accumulator (it merely duplicated `write_indices`). - Add an explanatory inline comment + docstring note so the canonical example teaches the correct **after-scatter** pattern. ## Verified - Matches the authoritative reference in `tests/static_cache_parity_test.py` (prefill nonpad = prompt_len = write_indices(0) + query_len, lines 322-327; decode nonpad = valid_len + 1 = write_indices(valid_len) + 1, lines 361-362) and the consumer `src/mobius/components/_attention.py:169-183` (the scatter precedes the maskless `is_causal` Attention's `nonpad` read). - Ran on CPU with `Qwen/Qwen2.5-0.5B` -> coherent output: `The capital of France is` -> ` Paris. It is the largest city in`. Pre-fix the all-zero prefill nonpad silently produced wrong logits. - Triple-reviewed (code / critical / readability all ship). ## Reference - onnx/onnx#8068 (causal bottom-right errata). - Relates to the static-cache + Flash enablement in PR #364 — this is a separate, example-only follow-up. --------- Signed-off-by: titaiwang <titaiwang@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6630bd3 commit a49e0f8

1 file changed

Lines changed: 13 additions & 8 deletions

File tree

examples/static_cache_generation.py

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@
1313
- No ``attention_mask`` input — causal masking is handled internally.
1414
- 3-D cache shape ``[batch, max_seq_len, kv_hidden]`` (not 4-D).
1515
- ``write_indices`` tracks where to write the next token's KV entry.
16-
- ``nonpad_kv_seqlen`` tracks how many valid KV entries exist.
16+
- ``nonpad_kv_seqlen`` is the number of valid KV entries after the current
17+
chunk is scattered into the cache (``write_indices + cur_seq_len``).
1718
- Outputs are ``updated_key_cache.{i}`` / ``updated_value_cache.{i}``.
1819
1920
Usage::
@@ -102,10 +103,6 @@ def generate(
102103
# starting from here); for decode steps it advances by 1 each step.
103104
write_indices = np.zeros((batch_size,), dtype=np.int64)
104105

105-
# nonpad_kv_seqlen: number of valid (non-padding) entries in the
106-
# cache so far. Starts at 0 before the first forward pass.
107-
nonpad_kv_seqlen = np.zeros((batch_size,), dtype=np.int64)
108-
109106
generated_ids: list[int] = []
110107

111108
for _step in range(max_new_tokens):
@@ -129,7 +126,14 @@ def generate(
129126
"input_ids": cur_input_ids,
130127
"position_ids": position_ids,
131128
"write_indices": write_indices,
132-
"nonpad_kv_seqlen": nonpad_kv_seqlen,
129+
# nonpad_kv_seqlen is the number of valid KV entries AFTER this
130+
# chunk is scattered into the cache (the model scatters K/V before
131+
# the maskless is_causal Attention reads nonpad_kv_seqlen). Under
132+
# the bottom-right contract that valid count must include the chunk
133+
# just written, so it is write_indices (entries already in cache)
134+
# + cur_seq_len (entries added by this chunk) — e.g. prompt_len at
135+
# prefill and write_indices + 1 on each decode step.
136+
"nonpad_kv_seqlen": write_indices + cur_seq_len,
133137
**cache,
134138
}
135139

@@ -153,9 +157,10 @@ def generate(
153157
cache[f"key_cache.{i}"] = outputs[f"updated_key_cache.{i}"]
154158
cache[f"value_cache.{i}"] = outputs[f"updated_value_cache.{i}"]
155159

156-
# Advance write position and valid-length counters
160+
# Advance write position for the next step. nonpad_kv_seqlen is
161+
# derived from write_indices at feed time (write_indices + cur_seq_len),
162+
# so there is no separate valid-length counter to maintain here.
157163
write_indices = write_indices + cur_seq_len
158-
nonpad_kv_seqlen = nonpad_kv_seqlen + cur_seq_len
159164

160165
# Decode one token at a time after the prefill step
161166
cur_input_ids = next_token.astype(np.int64)

0 commit comments

Comments
 (0)