Skip to content

Gemma3n and Gemma4 cannot use rotary kernel#45564

Merged
Cyrilvallez merged 1 commit into
mainfrom
gemmas-rotary
Apr 23, 2026
Merged

Gemma3n and Gemma4 cannot use rotary kernel#45564
Cyrilvallez merged 1 commit into
mainfrom
gemmas-rotary

Conversation

@Cyrilvallez
Copy link
Copy Markdown
Member

What does this PR do?

As per the title. Some layers need to apply the rotary only to q, so it cannot use the kernel.
cc @vasqu

@github-actions
Copy link
Copy Markdown
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: gemma3n, gemma4

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Copy Markdown
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can't standardize their rope for transpose etc?

Copy link
Copy Markdown
Contributor

@vasqu vasqu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good to reference #45489 (comment) for context @ArthurZucker

We would need to rewrite the kernels to allow single tensor application. Atm, this is not the case and hence not a valid usage for the kernels

@vasqu
Copy link
Copy Markdown
Contributor

vasqu commented Apr 22, 2026

Shouldn't be too hard to allow maybe 🤔 same for interleaved and partial layouts imo. It just needs to be properly defined

@Cyrilvallez Cyrilvallez merged commit bc4b330 into main Apr 23, 2026
23 checks passed
@Cyrilvallez Cyrilvallez deleted the gemmas-rotary branch April 23, 2026 01:50
tarekziade pushed a commit that referenced this pull request Apr 23, 2026
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.

4 participants