Skip to content

Commit de70209

Browse files
titaiwangmsCopilot
andcommitted
Address titaiwangms review: conditional nonpad invariant + inputs_embeds bias
PR #367 review fixes (no bias-math behavior change): MAJOR (doc accuracy + test): the cross-repo invariant is nonpad == write_indices + valid_token_count (UNPADDED query-token count), which equals write + S_q only when the chunk is unpadded (S_q is the PADDED chunk width). With intra-prompt padding + a sliding window, pad-token query rows can become fully masked. - create_static_cache_attention_bias docstring + the _apply_attention comment now state the CONDITIONAL invariant. - The 'CPU MEA returns a finite mean-of-V row, not NaN' claim is anchored to the empirically verified ORT 1.27 behavior (S_q=8, nonpad=3, window=4: rows 6-7 fully masked, stayed finite) rather than asserted as a permanent op-spec invariant. - Add test_fully_masked_row_stays_finite, which proves (via an independent numpy mask) that rows 6-7 are fully masked and asserts the CPU MEA output stays finite — the prior parity tests assert idx.size>0 and never reach this boundary. MINOR: _maybe_static_cache_bias no longer returns None when input_ids is None. It derives S_q from an always-present tensor (hidden_states), so inputs_embeds-driven forwards still receive the bias. NITS: - StaticCacheState docstring uses max_seq_len (was max_seq); the is_causal Note in _apply_attention is scoped to the DYNAMIC path (static mode ignores the incoming is_causal). - Guard the max_seq_len cast in _maybe_static_cache_bias: a symbolic key_cache KV dim now raises a descriptive TypeError naming the symbolic dim instead of an opaque int() failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: titaiwang <titaiwang@microsoft.com>
1 parent 4569cad commit de70209

4 files changed

Lines changed: 167 additions & 28 deletions

File tree

src/mobius/components/_attention.py

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ class StaticCacheState(NamedTuple):
6262
``nonpad_kv_seqlen`` to indicate valid token counts.
6363
6464
Fields:
65-
key_cache: Pre-allocated key cache [B, max_seq, kv_hidden] 3D.
66-
value_cache: Pre-allocated value cache [B, max_seq, kv_hidden] 3D.
65+
key_cache: Pre-allocated key cache [B, max_seq_len, kv_hidden] 3D.
66+
value_cache: Pre-allocated value cache [B, max_seq_len, kv_hidden] 3D.
6767
write_indices: Position to write new tokens [B] int64.
6868
nonpad_kv_seqlen: Valid KV length per batch entry [B] int64.
6969
"""
@@ -119,10 +119,12 @@ def _apply_attention(
119119
unmasking encoded in the bias.
120120
121121
Note:
122-
Both paths default to ``is_causal=1`` on the Attention op, which
123-
enables built-in causal masking. This means ``attn_mask`` should
124-
encode only padding information (as a bool mask), not causality,
125-
unless ``is_causal=0`` is passed explicitly.
122+
This applies to the DYNAMIC cache path only. There, the Attention op
123+
defaults to ``is_causal=1`` for built-in causal masking, so
124+
``attn_mask`` should encode only padding information (as a bool mask),
125+
not causality, unless ``is_causal=0`` is passed explicitly. In STATIC
126+
cache mode the incoming ``is_causal`` argument is ignored — causality
127+
is derived from ``attn_mask`` presence (see above).
126128
127129
Note:
128130
``nonpad_kv_seqlen`` (input #6) is only valid in static cache mode
@@ -171,14 +173,19 @@ def _apply_attention(
171173
# nonpad_kv_seqlen stays as input #6 in BOTH modes: it bounds the valid
172174
# KV prefix and, on the CUDA Flash path, drives the fully-masked-row
173175
# zero guard (LaunchZeroFullyMaskedRows). In bias mode the additive
174-
# bias already encodes the same ``slot < nonpad`` validity, and with the
175-
# contract-consistent feed (nonpad == write_indices + S_q) every query
176-
# row keeps its own diagonal slot valid — so a fully-masked
177-
# (all-``dtype.min``) row never arises in normal operation. Even if one
178-
# were forced (an out-of-contract feed), the CPU MEA path this bias mode
179-
# uses does NOT apply the Flash zero-guard: it returns a finite
180-
# mean-of-V row, not NaN and not exactly 0 (the zero guard is
181-
# CUDA-Flash-specific).
176+
# bias already encodes the same ``slot < nonpad`` validity. The
177+
# cross-repo invariant is ``nonpad == write_indices + valid_token_count``
178+
# (the count of UNPADDED query tokens), which equals
179+
# ``write_indices + S_q`` only when the chunk is unpadded — S_q is the
180+
# PADDED chunk width. When the chunk is unpadded, every query row keeps
181+
# its own diagonal slot valid, so a fully-masked (all-``dtype.min``) row
182+
# never arises. With intra-prompt padding plus a sliding window,
183+
# however, a pad-token query row CAN fall outside every valid slot and
184+
# become fully masked. In that case the CPU MEA path this bias mode uses
185+
# does NOT apply the Flash zero-guard: it returns a finite mean-of-V row
186+
# (not NaN, not exactly 0). This finite-row behavior was empirically
187+
# verified on ORT 1.27 CPU MEA; it is an observed ORT-version behavior,
188+
# not a permanent op-spec invariant — see test_fully_masked_row_stays_finite.
182189
if attn_mask is not None:
183190
mask_arg, causal = attn_mask, 0
184191
else:

src/mobius/components/_common.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,14 @@ def create_static_cache_attention_bias(
307307
(e.g. ``op.Shape(input_ids, start=1, end=2)``); squeezed to a scalar
308308
internally.
309309
nonpad_kv_seqlen: ``[B]`` int64 count of valid cache slots *after* the
310-
current chunk is scattered (``write_indices + S_q`` under the
311-
bottom-right contract).
310+
current chunk is scattered. The cross-repo invariant is
311+
``nonpad_kv_seqlen == write_indices + valid_token_count``, where
312+
``valid_token_count`` is the number of *unpadded* query tokens in the
313+
chunk. This equals ``write_indices + S_q`` only when the chunk is
314+
**unpadded** (``S_q`` is the padded chunk width). With intra-prompt
315+
padding plus a sliding window, pad-token query rows can fall outside
316+
every valid slot and become fully masked — see the fully-masked-row
317+
behavior note in ``_apply_attention``.
312318
max_seq_len: Static width of the pre-allocated cache KV axis.
313319
sliding_window: Optional local-attention window; when set, a query at
314320
absolute position ``q`` attends slot ``k`` only if ``q - k < w``.

src/mobius/models/base.py

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def __init__(self, config: ArchitectureConfig, mlp_class: type | None = None):
8484
def _maybe_static_cache_bias(
8585
self,
8686
op: OpBuilder,
87-
input_ids: ir.Value | None,
87+
seq_len_source: ir.Value,
8888
past_key_values: list | None,
8989
) -> ir.Value | None:
9090
"""Optionally build the static-cache float additive attention bias.
@@ -94,11 +94,18 @@ def _maybe_static_cache_bias(
9494
* the model declares a bias need (``self._sliding_window`` is set), AND
9595
* the cache is the opset-24 external cache (``StaticCacheState``).
9696
97-
When emitted, the bias is a ``(B, 1, S_q, max_seq)`` additive mask keyed
98-
on absolute query positions with KV validity ``slot < nonpad_kv_seqlen``;
99-
``_apply_attention`` then pairs it with ``is_causal=0``. The
100-
``write_indices`` / ``nonpad_kv_seqlen`` graph inputs are shared across
101-
all layers, so the first layer's cache state carries them.
97+
When emitted, the bias is a ``(B, 1, S_q, max_seq_len)`` additive mask
98+
keyed on absolute query positions with KV validity
99+
``slot < nonpad_kv_seqlen``; ``_apply_attention`` then pairs it with
100+
``is_causal=0``. The ``write_indices`` / ``nonpad_kv_seqlen`` graph
101+
inputs are shared across all layers, so the first layer's cache state
102+
carries them.
103+
104+
Args:
105+
seq_len_source: An always-present ``[B, S_q, ...]`` tensor (e.g.
106+
``hidden_states``) whose dim 1 is the query length ``S_q``. Using
107+
this instead of ``input_ids`` keeps the bias enabled for
108+
``inputs_embeds``-driven forwards (where ``input_ids`` is None).
102109
"""
103110
if not flags.static_cache_bias or self._sliding_window is None:
104111
return None
@@ -107,12 +114,20 @@ def _maybe_static_cache_bias(
107114
first = past_key_values[0]
108115
if not isinstance(first, StaticCacheState):
109116
return None
110-
if input_ids is None:
111-
return None
112117

113-
# Static cache KV axis width is a concrete int: [B, max_seq, kv_hidden].
114-
max_seq_len = int(first.key_cache.shape[1])
115-
seq_len = op.Shape(input_ids, start=1, end=2) # (1,) int64 == [S_q]
118+
# Static cache KV axis width is a concrete int: [B, max_seq_len, kv_hidden].
119+
# Guard against a symbolic dim, which would otherwise raise an opaque
120+
# TypeError downstream. Static-cache always allocates a fixed width today.
121+
max_seq_len = first.key_cache.shape[1]
122+
if not isinstance(max_seq_len, int):
123+
raise TypeError(
124+
"static-cache bias requires a concrete key_cache KV dimension "
125+
f"(axis 1), but got symbolic dim {max_seq_len!r}. The static "
126+
"cache must be allocated with a fixed max_seq_len."
127+
)
128+
# S_q lives at dim 1 of both input_ids ([B, S_q]) and hidden_states
129+
# ([B, S_q, hidden]), so the bias works for either forward entry point.
130+
seq_len = op.Shape(seq_len_source, start=1, end=2) # (1,) int64 == [S_q]
116131
return create_static_cache_attention_bias(
117132
op,
118133
write_indices=first.write_indices,
@@ -216,7 +231,9 @@ def forward(
216231
attention_mask=attention_mask,
217232
)
218233
else:
219-
attention_bias = self._maybe_static_cache_bias(op, input_ids, past_key_values)
234+
attention_bias = self._maybe_static_cache_bias(
235+
op, hidden_states, past_key_values
236+
)
220237

221238
present_key_values = []
222239
past_kvs = past_key_values or [None] * len(self.layers)

tests/static_cache_bias_parity_test.py

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,115 @@ def test_nonpad_padding_clamp_matches_dense_reference():
519519
np.testing.assert_allclose(onnx_attn, ref, atol=1e-4, rtol=1e-4)
520520

521521

522+
def _fully_masked_query_rows(
523+
*,
524+
query_len: int,
525+
write_idx: int,
526+
nonpad: int,
527+
sliding_window: int,
528+
max_seq_len: int,
529+
) -> list[int]:
530+
"""Return the query-row indices that are fully masked (zero valid slots).
531+
532+
Mirrors ``_dense_reference``'s rule order (causal AND sliding window AND
533+
padding validity, no block overlay) so a test can prove it genuinely
534+
exercises the all-masked-row boundary instead of silently skipping it.
535+
"""
536+
kv_slots = np.arange(max_seq_len)
537+
empty_rows: list[int] = []
538+
for t in range(query_len):
539+
q_abs = write_idx + t
540+
mask = q_abs >= kv_slots # causal
541+
mask &= (q_abs - kv_slots) < sliding_window # local window
542+
mask &= kv_slots < nonpad # padding validity
543+
if not mask.any():
544+
empty_rows.append(t)
545+
return empty_rows
546+
547+
548+
def test_fully_masked_row_stays_finite():
549+
"""Intra-prompt padding + sliding window can fully mask a pad-token row.
550+
551+
Counters the ``nonpad == write + S_q`` framing: ``S_q`` is the PADDED chunk
552+
width, while ``nonpad == write + valid_token_count`` uses the UNPADDED count.
553+
When the chunk carries trailing pad tokens (``nonpad < write + S_q``) AND a
554+
sliding window is active, the high pad-token query rows can fall outside
555+
every valid slot and become fully masked (all-``dtype.min`` bias row).
556+
557+
Unlike the other parity tests (which assert ``idx.size > 0`` via
558+
``_dense_reference`` and never reach this boundary), this test deliberately
559+
drives the empty-row case and asserts the CPU MEA external-cache path keeps
560+
the output FINITE (a finite mean-of-V row, not NaN, not exactly 0).
561+
562+
Scenario reproduces the reviewer's ORT 1.27 finding (``S_q=8, nonpad=3,
563+
window=4`` → rows 6-7 have zero valid slots and stay finite). This is an
564+
observed ORT-version behavior (verified on 1.27), not a permanent op-spec
565+
guarantee — see the note in ``_apply_attention``.
566+
"""
567+
query_len, write_idx, nonpad = 8, 0, 3
568+
569+
# Prove the config genuinely produces fully-masked rows (else the test would
570+
# silently pass without exercising the boundary it claims to cover).
571+
empty_rows = _fully_masked_query_rows(
572+
query_len=query_len,
573+
write_idx=write_idx,
574+
nonpad=nonpad,
575+
sliding_window=_SLIDING_WINDOW,
576+
max_seq_len=_MAX_SEQ_LEN,
577+
)
578+
assert empty_rows == [6, 7], (
579+
f"expected rows 6-7 fully masked for the reviewer's scenario, got {empty_rows}"
580+
)
581+
582+
rng = np.random.default_rng(11)
583+
q_hidden = _NUM_Q_HEADS * _HEAD_DIM
584+
kv_hidden = _NUM_KV_HEADS * _HEAD_DIM
585+
query = rng.standard_normal((_BATCH, query_len, q_hidden)).astype(np.float32)
586+
key = rng.standard_normal((_BATCH, query_len, kv_hidden)).astype(np.float32)
587+
value = rng.standard_normal((_BATCH, query_len, kv_hidden)).astype(np.float32)
588+
key_cache = np.zeros((_BATCH, _MAX_SEQ_LEN, kv_hidden), dtype=np.float32)
589+
value_cache = np.zeros((_BATCH, _MAX_SEQ_LEN, kv_hidden), dtype=np.float32)
590+
write_indices = np.full((_BATCH,), write_idx, dtype=np.int64)
591+
nonpad_kv_seqlen = np.full((_BATCH,), nonpad, dtype=np.int64)
592+
593+
model = _build_static_cache_bias_graph(
594+
batch=_BATCH,
595+
num_q_heads=_NUM_Q_HEADS,
596+
num_kv_heads=_NUM_KV_HEADS,
597+
head_dim=_HEAD_DIM,
598+
max_seq_len=_MAX_SEQ_LEN,
599+
query_len=query_len,
600+
sliding_window=_SLIDING_WINDOW,
601+
use_block_overlay=False,
602+
)
603+
session = OnnxModelSession(model) # CPU EP (MEA external-cache path)
604+
try:
605+
out = session.run(
606+
{
607+
"query": query,
608+
"key": key,
609+
"value": value,
610+
"key_cache": key_cache,
611+
"value_cache": value_cache,
612+
"write_indices": write_indices,
613+
"nonpad_kv_seqlen": nonpad_kv_seqlen,
614+
}
615+
)
616+
finally:
617+
session.close()
618+
619+
attn_output = out["attn_output"]
620+
# The core empirically-verified claim: even fully-masked rows stay finite
621+
# (no NaN/Inf) on the CPU MEA path — the Flash zero-guard is not applied.
622+
assert np.isfinite(attn_output).all(), (
623+
"fully-masked query row produced NaN/Inf on the CPU MEA external-cache path"
624+
)
625+
for t in empty_rows:
626+
assert np.isfinite(attn_output[:, t, :]).all(), (
627+
f"fully-masked query row {t} is not finite"
628+
)
629+
630+
522631
@pytest.mark.parametrize("sliding_window", [None, 2, 4])
523632
def test_prefill_sliding_window_variants(sliding_window):
524633
"""Prefill parity across no-window and tight/loose sliding windows.

0 commit comments

Comments
 (0)