Skip to content

[Relax] Fix llama4_rope_with_position_map to support partial rotary factor#18520

Merged
MasterJH5574 merged 1 commit intoapache:mainfrom
guan404ming:fix-phi
Feb 12, 2026
Merged

[Relax] Fix llama4_rope_with_position_map to support partial rotary factor#18520
MasterJH5574 merged 1 commit intoapache:mainfrom
guan404ming:fix-phi

Conversation

@guan404ming
Copy link
Member

@guan404ming guan404ming commented Nov 28, 2025

Related Issue

closes #17715

Why

  • Phi-4 uses: partial_rotary_factor = 0.75 (rotary_dim = 96) + longrope scaling
  • Longrope requires: Both long_factors + short_factors packed into one buffer
  • Expected buffer size: (rotary_dim,) = (96,) total
    • First half [0:48] = long_factors
    • Second half [48:96] = short_factors
  • llama4_rope_with_position_map still had old size (rotary_dim // 2,) = (48,)

@guan404ming guan404ming changed the title Fix llama4_rope_with_position_map to support partial rotary factor [Relax] Fix llama4_rope_with_position_map to support partial rotary factor Nov 28, 2025
@guan404ming guan404ming marked this pull request as ready for review November 28, 2025 07:52
@guan404ming
Copy link
Member Author

cc @tlopex @mshr-h

@tlopex
Copy link
Member

tlopex commented Nov 28, 2025

cc @MasterJH5574

Copy link
Contributor

@MasterJH5574 MasterJH5574 left a comment

Choose a reason for hiding this comment

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

Thank you for the fix. The changes look good, and we will help rebase and merge it.

Copy link
Contributor

@MasterJH5574 MasterJH5574 left a comment

Choose a reason for hiding this comment

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

Rebased. Thanks for the fix.

@MasterJH5574 MasterJH5574 merged commit 1bd40fa into apache:main Feb 12, 2026
10 checks passed
@guan404ming
Copy link
Member Author

Thanks!

@guan404ming guan404ming deleted the fix-phi branch February 13, 2026 02:14
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.

[Bug] Inference - Phi-4 mini instruct

3 participants