Skip to content

fix(models): temporarily disable TE fused RoPE globally#3028

Merged
akoumpa merged 5 commits into
mainfrom
huiyingl/fix/disable-rope-fusion
Jul 12, 2026
Merged

fix(models): temporarily disable TE fused RoPE globally#3028
akoumpa merged 5 commits into
mainfrom
huiyingl/fix/disable-rope-fusion

Conversation

@HuiyingLi

Copy link
Copy Markdown
Contributor

What

Force rope_fusion=False for every BackendConfig in __post_init__, disabling TE fused RoPE globally.

Why

The fused RoPE kernel computes cos/sin in fp32 in-kernel while HF/vLLM rotate with bf16 tables, breaking logprob parity in some models (e.g. Qwen3-MoE). Temporary correctness-over-speed workaround until a parity fix lands — see #3027.

Notes

  • Single chokepoint: overrides both the default and any explicit rope_fusion: true in a recipe/config; logs a one-time rank-0 warning when overriding an explicit True.
  • To re-enable, remove the block in BackendConfig.__post_init__.

🤖 Generated with Claude Code

Force rope_fusion=False in BackendConfig.__post_init__ so no model family
uses TE's fused RoPE kernel. The fused kernel computes cos/sin in fp32
in-kernel while HF/vLLM rotate with bf16 tables, breaking logprob parity
in some models. Temporary workaround; see #3027.

Signed-off-by: HuiyingLi <willwin.lee@gmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: HuiyingLi <willwin.lee@gmail.com>
@HuiyingLi HuiyingLi requested a review from a team as a code owner July 11, 2026 05:53
@copy-pr-bot

copy-pr-bot Bot commented Jul 11, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Add TestBackendConfigRopeFusionDisabled: default resolves False, explicit
True is overridden (with warning), explicit False stays False. See #3027.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: HuiyingLi <willwin.lee@gmail.com>
@HuiyingLi

Copy link
Copy Markdown
Contributor Author

/claude review

@HuiyingLi

Copy link
Copy Markdown
Contributor Author

/ok to test cbcaeeb

@HuiyingLi

Copy link
Copy Markdown
Contributor Author

/claude review

Comment thread nemo_automodel/components/models/common/utils.py Outdated
Comment thread nemo_automodel/components/models/common/utils.py Outdated
@akoumpa

akoumpa commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

/ok to test 958cedc

akoumpa
akoumpa previously approved these changes Jul 11, 2026
The qwen3_moe / deepseek_v3 context-parallel thd_te tests built RoPE
frequencies with for_fused_rope=True while relying on the default
BackendConfig.rope_fusion. With fused RoPE now force-disabled globally
(see #3027), the attention takes the non-fused path but the fused-format
freqs no longer match -> thd_te parity check fails. Derive for_fused_rope
from the resolved backend.rope_fusion so freqs and attention stay
consistent; this exercises the non-fused CP path that production already
uses for cp_size>1, and auto-restores to fused when #3027 is reverted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: HuiyingLi <willwin.lee@gmail.com>
@HuiyingLi

Copy link
Copy Markdown
Contributor Author

/ok to test f2197b8

@akoumpa akoumpa merged commit ae66147 into main Jul 12, 2026
81 checks passed
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.

Temporarily disable TE fused RoPE globally (breaks HF/vLLM logprob parity in some models)

2 participants