Skip to content

Rename variables in rotary embedding reference for clarity#7316

Merged
titaiwangms merged 4 commits intoonnx:mainfrom
titaiwangms:titaiwang/naming_fixes_on_rotary_embedding
Sep 22, 2025
Merged

Rename variables in rotary embedding reference for clarity#7316
titaiwangms merged 4 commits intoonnx:mainfrom
titaiwangms:titaiwang/naming_fixes_on_rotary_embedding

Conversation

@titaiwangms
Copy link
Copy Markdown
Contributor

@titaiwangms titaiwangms commented Sep 22, 2025 •
edited by justinchuby
Loading

Address comments #7313 (comment)

@titaiwangms titaiwangms requested a review from a team as a code owner September 22, 2025 21:21
@github-project-automation github-project-automation Bot moved this to In progress in PR Tracker Sep 22, 2025
@titaiwangms
rename variables
ea045b6 
Signed-off-by: Ti-Tai Wang <titaiwang@microsoft.com>
@titaiwangms
update doc
852a547 
Signed-off-by: Ti-Tai Wang <titaiwang@microsoft.com>
@titaiwangms
indent
bd72a88 
Signed-off-by: Ti-Tai Wang <titaiwang@microsoft.com>
@titaiwangms titaiwangms force-pushed the titaiwang/naming_fixes_on_rotary_embedding branch from 6f457a5 to bd72a88 Compare September 22, 2025 21:23
@titaiwangms titaiwangms mentioned this pull request Sep 22, 2025
Merged
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 22, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.33%. Comparing base (b849e09) to head (b36b196).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
onnx/reference/ops/op_rotary_embedding.py 75.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #7316      +/-   ##
==========================================
- Coverage   54.34%   54.33%   -0.01%     
==========================================
  Files         511      511              
  Lines       31839    31837       -2     
  Branches     2852     2852              
==========================================
- Hits        17302    17300       -2     
  Misses      13757    13757              
  Partials      780      780

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

@titaiwangms
update docs
b36b196 
Signed-off-by: Ti-Tai Wang <titaiwang@microsoft.com>
@github-project-automation github-project-automation Bot moved this from In progress to Reviewer approved in PR Tracker Sep 22, 2025
@justinchuby
Copy link
Copy Markdown
Member

justinchuby commented Sep 22, 2025

cc @gramalingam

@titaiwangms titaiwangms enabled auto-merge (squash) September 22, 2025 22:20
@titaiwangms titaiwangms merged commit d2813e1 into onnx:main Sep 22, 2025
49 checks passed
@github-project-automation github-project-automation Bot moved this from Reviewer approved to Done in PR Tracker Sep 22, 2025
justinchuby pushed a commit that referenced this pull request Sep 22, 2025
@titaiwangms @justinchuby
Rename variables in rotary embedding reference for clarity (#7316)
f559ffe 
Address comments
#7313 (comment)

---------

Signed-off-by: Ti-Tai Wang <titaiwang@microsoft.com>
justinchuby added a commit that referenced this pull request Sep 22, 2025
@justinchuby @titaiwangms
Update rotary_embedding reference implementation and tests (#7304, #7316
6130272 
) (#7313)

Cherrypick of 

- #7304
- #7316

---------

Signed-off-by: Ti-Tai Wang <titaiwang@microsoft.com>
Co-authored-by: Ti-Tai Wang <titaiwang@microsoft.com>
@yuanyao-nv yuanyao-nv mentioned this pull request Oct 9, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gramalingam gramalingam gramalingam approved these changes

@justinchuby justinchuby justinchuby approved these changes

Assignees

No one assigned

Labels

module: reference implementation

Projects

PR Tracker
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@titaiwangms @justinchuby @gramalingam