Skip to content

Conversation

gramalingam
Copy link
Collaborator

Fix Onnx 23 Rotary Fusion

Signed-off-by: Ganesan Ramalingam <[email protected]>
Copy link

codecov bot commented Sep 25, 2025

Codecov Report

❌ Patch coverage is 84.90566% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.04%. Comparing base (168fd8a) to head (a6fd40c).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...xscript/rewriter/rules/fusion/_rotary_embedding.py 78.94% 2 Missing and 2 partials ⚠️
...pt/rewriter/rules/fusion/_rotary_embedding_test.py 86.66% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2576      +/-   ##
==========================================
- Coverage   70.10%   70.04%   -0.06%     
==========================================
  Files         222      223       +1     
  Lines       26184    26215      +31     
  Branches     2581     2583       +2     
==========================================
+ Hits        18355    18363       +8     
- Misses       6929     6946      +17     
- Partials      900      906       +6     

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

Signed-off-by: Ganesan Ramalingam <[email protected]>
@gramalingam gramalingam changed the title DRAFT: Fix Onnx 23 Rotary Fusion Fix Onnx 23 Rotary Fusion Sep 25, 2025
@gramalingam gramalingam marked this pull request as ready for review September 25, 2025 20:02
@justinchuby
Copy link
Collaborator

justinchuby commented Sep 25, 2025

Nice: looks like the values now match

(tensor([[[-1.0310,  0.0658, -0.1172,  ...,  0.2810, -0.0586, -0.4056],
         [ 1.5216, -2.2804,  2.4418,  ...,  0.6617, -2.2747,  1.3125],
         [ 0.3216, -0.1102, -0.7690,  ..., -2.1603, -2.1465, -0.0737]],

        [[-0.5299,  0.3161, -0.0311,  ...,  0.5096,  1.7270,  0.2317],
         [-0.0963,  0.4574, -1.3810,  ..., -1.6085,  3.8896,  1.3088],
         [ 0.5815, -2.3405,  0.8283,  ...,  1.8330, -0.1297,  0.9608]]]),)
------------
Gemma3ModelOutputWithPast(last_hidden_state=
tensor([[[-1.0310,  0.0658, -0.1172,  ...,  0.2810, -0.0586, -0.4056],
         [ 1.5215, -2.2804,  2.4418,  ...,  0.6617, -2.2747,  1.3125],
         [ 0.3216, -0.1102, -0.7690,  ..., -2.1603, -2.1465, -0.0737]],

        [[-0.5299,  0.3161, -0.0311,  ...,  0.5096,  1.7270,  0.2317],
         [-0.0967,  0.4576, -1.3808,  ..., -1.6082,  3.8898,  1.3089],
         [ 0.5815, -2.3406,  0.8283,  ...,  1.8331, -0.1296,  0.9608]]],
       grad_fn=<MulBackward0>), past_key_values=None, hidden_states=None, attentions=None, image_hidden_states=None)

@justinchuby
Copy link
Collaborator

Hmm. It doesn't seem right: There is no rotary nodes in the graph
image

Signed-off-by: Ganesan Ramalingam <[email protected]>
Signed-off-by: Ganesan Ramalingam <[email protected]>
@gramalingam gramalingam enabled auto-merge (squash) September 26, 2025 18:31
onnx_version = Version(onnx.__version__)
min_version = Version("1.19.1")
is_stable = not (onnx_version.is_devrelease or onnx_version.is_prerelease)
if onnx_version >= min_version and is_stable:

Check failure

Code scanning / CodeQL

Potentially uninitialized local variable Error

Local variable 'is_stable' may be used before it is initialized.

Copilot Autofix

AI 5 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

@gramalingam gramalingam merged commit dddf0c2 into main Sep 26, 2025
31 of 32 checks passed
@gramalingam gramalingam deleted the rama/rotary branch September 26, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants