Skip to content

Update rotary_embedding reference implementation and tests (#7304, #7316)#7313

Merged
justinchuby merged 3 commits intorel-1.19.1from
justinchu/pick-be00bb
Sep 22, 2025
Merged

Update rotary_embedding reference implementation and tests (#7304, #7316)#7313
justinchuby merged 3 commits intorel-1.19.1from
justinchu/pick-be00bb

Conversation

@justinchuby
Copy link
Copy Markdown
Member

@justinchuby justinchuby commented Sep 22, 2025

### Description
<!-- - Describe your changes. -->

Add assertion in reference op of rotary embedding to follow ONNX spec.

### Motivation and Context
<!-- - Why is this change required? What problem does it solve? -->
<!-- - If it fixes an open issue, please link to the issue here. -->

In ONNX spec, `cos_cache` and `sin_cache` are explicitly stated to have
either rotary_embedding_dim/2 or head_size/2 in the final dim. However,
reference op did not force it.

---------

Signed-off-by: Ti-Tai Wang <titaiwang@microsoft.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 22, 2025

Codecov Report

❌ Patch coverage is 37.50000% with 10 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (rel-1.19.1@fb51738). Learn more about missing BASE report.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
onnx/backend/test/case/node/rotaryembedding.py 0.00% 6 Missing ⚠️
onnx/reference/ops/op_rotary_embedding.py 60.00% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             rel-1.19.1    #7313   +/-   ##
=============================================
  Coverage              ?   53.75%           
=============================================
  Files                 ?      512           
  Lines                 ?    32214           
  Branches              ?     2948           
=============================================
  Hits                  ?    17318           
  Misses                ?    14123           
  Partials              ?      773           

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

@justinchuby justinchuby enabled auto-merge (squash) September 22, 2025 20:04
Comment thread onnx/reference/ops/op_rotary_embedding.py
@github-project-automation github-project-automation Bot moved this from In progress to Reviewer approved in PR Tracker Sep 22, 2025
titaiwangms added a commit that referenced this pull request Sep 22, 2025
Address comments
#7313 (comment)

---------

Signed-off-by: Ti-Tai Wang <titaiwang@microsoft.com>
Address comments
#7313 (comment)

---------

Signed-off-by: Ti-Tai Wang <titaiwang@microsoft.com>
@justinchuby justinchuby changed the title Update rotary_embedding reference implementation and tests (#7304) Update rotary_embedding reference implementation and tests (#7304, #7316) Sep 22, 2025
@justinchuby justinchuby enabled auto-merge (squash) September 22, 2025 23:23
@justinchuby justinchuby merged commit 6130272 into rel-1.19.1 Sep 22, 2025
35 of 36 checks passed
@justinchuby justinchuby deleted the justinchu/pick-be00bb branch September 22, 2025 23:36
@github-project-automation github-project-automation Bot moved this from Reviewer approved to Done in PR Tracker Sep 22, 2025
@yuanyao-nv yuanyao-nv mentioned this pull request Oct 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants