Fix CPU Attention causal mask alignment - #29050
Conversation
|
#28958 tried to fix the same issue? |
|
I checked the #28958 diff. It appears to target the opset-24 For the #29020 repro path, where there is no bool causal = parameters.is_causal && parameters.q_sequence_length > 1;So for This PR fixes that separate no- |
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
This PR aligns the CPU Execution Provider’s opset-23 Attention causal masking behavior with the ONNX spec’s upper-left alignment for the q_len=1, kv_len>1, no past case (matching CUDA), and updates the Python reference implementation plus tests to reflect the same semantics. It also adds a targeted regression test for the CPU/CUDA divergence reported in issue #29020.
Changes:
- Fix CPU causal-mask gating so
is_causal=1is honored forq_sequence_length==1when there is no past. - Update the Python attention reference causal mask to model ONNX “upper-left” alignment, with an explicit
past_seqlenoffset. - Add a regression test that fails if CPU incorrectly allows the single query to attend to all KV positions in the no-past cross-attention case.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| onnxruntime/core/providers/cpu/llm/attention.cc | Adjusts causal-mask enablement so q_len==1 no longer disables causal masking when past_sequence_length==0. |
| onnxruntime/test/providers/cpu/llm/attention_op_test.cc | Adds Attention4DCausalSingleQueryCrossAttentionUpperLeft to prevent CPU/CUDA divergence for q_len=1, kv_len>1, no past. |
| onnxruntime/test/python/transformers/test_onnx_attention/common.py | Updates reference causal mask to ONNX upper-left behavior and adds past_seqlen to correctly offset decode masking. |
| onnxruntime/test/python/transformers/test_onnx_attention/test_mha.py | Passes past_seqlen into the Python reference attention for decode/past parity checks. |
| onnxruntime/test/python/transformers/test_onnx_attention/test_gqa.py | Passes past_seqlen into the Python reference attention for GQA decode/past parity checks (including padding path). |
|
CI workflows are pending maintainer approval for the latest commit. |
|
The merge conflicts are resolved now. Please continue the review when convenient. |
titaiwangms
left a comment
There was a problem hiding this comment.
🟢 Minor / nits (non-blocking): add a comment explaining why the nonpad branch drops causal at q=1; add a CPU regression test for nonpad+causal+q=1 ; the new test ( y all-ones vs 1/6 unfixed) is correctly discriminative.
|
Thanks. I added a comment explaining why the nonpad q_len=1 path keeps causal disabled, and added a CPU regression test for nonpad + causal + q_len=1. The new test expects 1/6 for bottom-right decode behavior; applying the no-nonpad upper-left overlay would produce 1.0 instead. |

Summary
nonpad_kv_seqlen/ TensorScatter single-query causal behaviorFixes #29020
Validation
python -m py_compile onnxruntime/test/python/transformers/test_onnx_attention/common.py onnxruntime/test/python/transformers/test_onnx_attention/test_mha.py onnxruntime/test/python/transformers/test_onnx_attention/test_gqa.py onnxruntime/test/python/transformers/test_onnx_attention/test_tensorscatter_attention.pygit diff --checkNotes:
pytest onnxruntime/test/python/transformers/test_onnx_attention/test_tensorscatter_attention.py -k "cpu_fp32 and causal" -qcould not run locally because this Python environment does not haveonnx/onnxruntimeinstalled.onnxruntime_provider_testwas attempted but failed in MSBuild before compiling this change due to a local environment issue: duplicatePath/PATHenvironment keys when launchingCL.exe.