Skip to content

docs: fix CUDA Attention skill-doc guidance to match bottom-right causal + fully-masked errata - #28988

Closed
titaiwangms wants to merge 1 commit into
mainfrom
titaiwang/fix-attention-skill-doc
Closed

docs: fix CUDA Attention skill-doc guidance to match bottom-right causal + fully-masked errata#28988
titaiwangms wants to merge 1 commit into
mainfrom
titaiwang/fix-attention-skill-doc

Conversation

@titaiwangms

Copy link
Copy Markdown
Contributor

What

Docs-only correction to the hand-written agent skill doc .agents/skills/cuda-attention-kernel-patterns/SKILL.md. No code or kernel change.

Why

The skill doc gave guidance opposite to the behavior shipped in #28958 (which implements the onnx/onnx#8068 errata). It would actively misdirect agents working on the ONNX-domain Attention op:

  • is_causal=1 + external/static KV cache (nonpad_kv_seqlen, no past_key): the doc called this spec-invalid and told models to use is_causal=0. It is in fact valid and supported, using bottom-right (offset-aware) alignment — offset[b] = nonpad_kv_seqlen[b] - q_sequence_length (clamped >= 0). Corrected, including the bottom-right key-range bullet that was written backwards.
  • Fully-masked query rows → Y = 0 on BOTH EPs (CPU Bug-2 guard + CUDA ZeroFullyMaskedRowsKernel). Removed the stale mean(V) 'spec reference' claim and the now-resolved cross-EP TODO.
  • qk_matmul_output mode-3: documented fully-masked row = 0 (mandated, consistent with Y), and that CUDA returns NOT_IMPLEMENTED for mode-3 (it is a CPU-only path).

Grounding

Matches the landed kernel: core/providers/cuda/llm/attention.cc (bottom-right dispatch, mode-3 NOT_IMPLEMENTED), attention_mask_impl.cu (ZeroFullyMaskedRowsKernel), core/providers/cpu/llm/attention.cc (Bug-2 row-zeroing). See #28958 and onnx/onnx#8068.

Risk

Documentation only — no build/runtime impact.

… fully-masked Y=0 + mode-3=0

The cuda-attention-kernel-patterns skill doc gave guidance opposite to the
shipped behavior in #28958 (onnx/onnx#8068 errata):

- is_causal=1 with an external/static KV cache (nonpad_kv_seqlen, no past_key)
  uses bottom-right (offset-aware) alignment and IS valid for decode — the doc
  previously called it spec-invalid and told models to use is_causal=0.
- Fully-masked query rows output Y=0 on BOTH EPs (CPU Bug-2 guard + CUDA
  ZeroFullyMaskedRowsKernel); removed the stale mean(V) 'spec reference' claim
  and the resolved cross-EP TODO.
- Documented qk_matmul_output mode-3 fully-masked row = 0 (mandated, consistent
  with Y), and that CUDA returns NOT_IMPLEMENTED for mode-3 (CPU-only path).
- Corrected the bottom-right key-range bullet (was backwards) to the
  j <= i + offset form matching the rest of the doc and the kernel.

Docs-only; no code/kernel change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Ti-Tai Wang <titaiwang@microsoft.com>
@titaiwangms

Copy link
Copy Markdown
Contributor Author

Folding this docs change into #28958 since it documents the same kernel behavior (bottom-right is_causal, fully-masked Y=0, mode-3=0) — keeping it atomic with the kernel change for review/merge. Closing in favor of #28958, which now carries the SKILL.md correction as a dedicated docs commit (1964d9b).

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