Skip to content

[gemma] Allow packed QKV MatMul with QK-Norm via post-projection Split - #366

Merged
xadupre merged 3 commits into
mainfrom
copilot/import-changes-from-onnxruntime-genai
May 25, 2026
Merged

[gemma] Allow packed QKV MatMul with QK-Norm via post-projection Split#366
xadupre merged 3 commits into
mainfrom
copilot/import-changes-from-onnxruntime-genai

Conversation

Copilot AI commented May 24, 2026

Copy link
Copy Markdown
Contributor

Ports microsoft/onnxruntime-genai#2137: previously use_packed_matmul was disabled whenever q_norm/k_norm was set, emitting 3 separate q_proj/k_proj/v_proj MatMulNBits per layer for Qwen3-class (QK-Norm) architectures. We now keep the single packed qkv_proj/MatMul and recover Q/K/V via one Split node so the per-head SimplifiedLayerNormalization path is unchanged.

Changes

  • base.pymake_attention_init: cache self.q_size / self.kv_size; drop q_norm/k_norm from the use_packed_matmul exclusion list.
  • base.pymake_split helper: new thin wrapper around ONNX Split.
  • base.pymake_attention_input_proj: when packed QKV is active and both Q/K norms are present, insert Split (widths [q_size, kv_size, kv_size]) after the optional packed bias Add (so packed-bias fusion is preserved) and rebind q_path/k_path/v_path to its 3 outputs. Diagram comments added.
  • base.py: replace inline num_attn_heads*head_size / num_kv_heads*head_size with cached q_size/kv_size in make_qk_norm, make_repeat_kv, make_attention_unpacked_lora, make_attention_unpacked_regular.
  • gemma.pyGemma4Model.make_attention: also swap (and restore) q_size/kv_size alongside the existing per-layer head_size / num_kv_heads swap, so full-attention layers get correct Split widths and SLN reshape dims.

Resulting subgraph (QK-Norm + packed)

QKV_MatMul → QKV_Add (packed, if bias) → Split → [Q_Norm, K_Norm, V]
                                                   → Q_Rotary / K_Rotary → GroupQueryAttention

A single Split is preferred over 3 Slice nodes: the packed output is read once and written to 3 outputs in one dispatch, avoiding 3× re-reads per decode step.

Copilot AI changed the title [WIP] Import changes from onnxruntime-genai pull 2137 and ensure tests pass Allow packed QKV MatMul with QK-Norm via post-projection Split May 24, 2026
Copilot AI requested a review from xadupre May 24, 2026 22:40
@xadupre
xadupre marked this pull request as ready for review May 24, 2026 22:42
@github-actions

github-actions Bot commented May 24, 2026

Copy link
Copy Markdown

📂 Impacted Files

2 file(s) changed in this pull request:

🔵 modelbuilder/builders/base.py (+84 / -12)
🔵 modelbuilder/builders/gemma.py (+6 / -0)

@github-actions github-actions Bot changed the title Allow packed QKV MatMul with QK-Norm via post-projection Split [gemma] Allow packed QKV MatMul with QK-Norm via post-projection Split May 24, 2026
@codecov

codecov Bot commented May 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.15385% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.44%. Comparing base (3f9dfcd) to head (f19d412).

Files with missing lines Patch % Lines
modelbuilder/builders/base.py 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #366      +/-   ##
==========================================
+ Coverage   78.39%   78.44%   +0.05%     
==========================================
  Files          26       26              
  Lines        7984     8003      +19     
==========================================
+ Hits         6259     6278      +19     
  Misses       1725     1725              
Flag Coverage Δ
fast-tests 78.44% <96.15%> (+0.05%) ⬆️
fast-tests-ort-nightly 78.39% <96.15%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xadupre

xadupre commented May 25, 2026

Copy link
Copy Markdown
Owner

@copilot one test on phi3 is failing with transformers 5.6

Copilot AI commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot one test on phi3 is failing with transformers 5.6

I'm unable to reproduce a phi3 failure locally with the exact CI dependency versions (transformers 5.6.0, onnxruntime 1.26.0, onnxruntime-genai 0.13.2, torch 2.11). All 26 test_random_phi3* tests pass at b842158, and my changes don't touch any phi3 code path (phi3 has no q_norm/k_norm so neither the new Split branch nor the use_packed_matmul exclusion change apply to it).

The MCP log endpoint returns 404 for that job, and the workflow-run logs zip on this PR snapshots the steps before "Run fast tests" finishes, so I cannot see which specific phi3 test failed or its traceback. Could you paste the failing test name + stack trace (or the relevant pytest output)? With that I can target the fix.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/repos/xadupre/mbext/actions/jobs/77633104896/logs
    • Triggering command: /usr/bin/curl curl -sLI REDACTED (http block)
  • https://api.github.com/repos/xadupre/mbext/actions/runs/26374761498/attempts/1/jobs
    • Triggering command: /usr/bin/curl curl -sL REDACTED (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from xadupre May 25, 2026 06:10
@xadupre

xadupre commented May 25, 2026

Copy link
Copy Markdown
Owner

@copilot can you try again?

Copilot AI commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot can you try again?

Tried again. Reinstalled the exact CI dep set (transformers==5.6.0, onnxruntime==1.26.0, onnxruntime-genai==0.13.2, onnx==1.21.0, peft==0.19.1) and ran the full tests/fast suite at b842158: 293 passed, 146 skipped — no failure on phi3 or anywhere else.

Inspecting the failing CI run (26374761498, job 77633104896, transformers-5.6) more carefully: the workflow-run log archive contains files 1_Set up job.txt through 7_pip freeze.txt, but no 8_Run fast tests.txt, and the API reports step 8 as in_progress while the job concluded as failure. That means the runner/step died before flushing any pytest output, which is almost always an infrastructure issue (OOM / runner eviction), not a code failure — and matches why the parallel transformers-5.9 job was force-cancelled mid-test_random_phi4_multimodal rather than ever reaching a real failure.

Could you re-run that job? If it fails again with the same pattern (no step-8 log), it's a runner issue; if a real test failure surfaces with a traceback, I can target the fix.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • download-r2.pytorch.org
    • Triggering command: /usr/bin/pip pip install -q torch==2.11.0&#43;cpu torchvision torchaudio --index-url REDACTED (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@xadupre

xadupre commented May 25, 2026

Copy link
Copy Markdown
Owner

@copilot the test python tests/fast/test_random_phi3.py works with transformers 5.9 but not 5.6 but it used to work with 5.6 so I assume it is a change due to transformers

Copilot AI commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot the test python tests/fast/test_random_phi3.py works with transformers 5.9 but not 5.6 but it used to work with 5.6 so I assume it is a change due to transformers

Just re-ran python tests/fast/test_random_phi3.py at HEAD (b842158) with transformers 5.6.0 (+ onnxruntime 1.26.0, onnxruntime-genai 0.13.2, onnx 1.21.0, torch 2.12, peft 0.19.1) — all 10 tests pass (Ran 10 tests in 6.831s — OK (skipped=4)), and pytest agrees (6 passed, 4 skipped). So I still can't reproduce the failure on this branch, and nothing in this PR touches the phi3 code path.

If you can share the exact pytest output / traceback from your local 5.6 run (or an exact pip freeze of the failing environment), I'll dig in — but right now the only signal I have is "passes here", so I don't have anything to act on.

@xadupre
xadupre merged commit 6cbf79d into main May 25, 2026
10 of 11 checks passed
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.

import changes from https://github.com/microsoft/onnxruntime-genai/pull/2137, make sure it passes the tests

2 participants