Skip to content

[https://nvbugs/6422318][fix] Cast cos_sin_cache to float32 at cat-time to satisfy flashinfer's contract…#16069

Open
trtllm-agent wants to merge 1 commit into
NVIDIA:mainfrom
tensorrt-cicd:repair-bot-bug6422318
Open

[https://nvbugs/6422318][fix] Cast cos_sin_cache to float32 at cat-time to satisfy flashinfer's contract…#16069
trtllm-agent wants to merge 1 commit into
NVIDIA:mainfrom
tensorrt-cicd:repair-bot-bug6422318

Conversation

@trtllm-agent

@trtllm-agent trtllm-agent commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Root cause: cos_sin_cache passed to flashinfer_apply_rope_with_cos_sin_cache_inplace inherits bf16 from the upstream vision-tower dtype cast, but flashinfer strictly requires float32.
  • Fix: Cast cos_sin_cache to float32 at cat-time to satisfy flashinfer's contract, broaden the fallback to also catch ValueError, and remove the B300/GB200/GB300/H20 waivers for this bug.
  • Automated fix generated by repair-bot

Test plan

  • Verify fix on the same GPU type as the original failure
  • Check for regressions in related tests

Links

Summary by CodeRabbit

  • Bug Fixes
    • Improved multimodal model stability by making the rotary embedding fast path more robust.
    • Added a safer fallback when the optimized path fails, reducing the chance of runtime errors.
    • Ensured intermediate values use a consistent numeric format for better compatibility.

…ly_rope

Flashinfer's apply_rope_with_cos_sin_cache_inplace strictly requires float32
cos_sin_cache, but the upstream _get_window_rope_pos_embeddings pre-casts
cos/sin to the vision-tower dtype (bf16 under auto_dtype) as a perf hint for
the PyTorch RoPE fallback. Qwen2.5-VL vision head_dim=80 skips the flashinfer
branch, but EXAONE-4.5 reuses the tower with a head_dim that is a multiple
of 64 and hits the strict dtype check.

Cast cos_sin_cache to float32 at cat-time so the flashinfer contract is met
without disturbing the upstream perf cast. Also broaden the exception clause
from RuntimeError to (RuntimeError, ValueError) so any future flashinfer
contract change falls back to the PyTorch path instead of failing the run.

Remove the corresponding B300/GB200/GB300/H20 waivers in waives.txt.

Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: fa5860ad-bd0a-4cdd-93dc-c607b5c7c743

📥 Commits

Reviewing files that changed from the base of the PR and between ce67288 and e413551.

📒 Files selected for processing (2)
  • tensorrt_llm/_torch/models/modeling_qwen2vl.py
  • tests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
  • tests/integration/test_lists/waives.txt

📝 Walkthrough

Walkthrough

This PR updates the FlashInfer RoPE fast-path in Qwen2.5-VL vision attention to cast the concatenated cos_sin_cache to float32 and broadens exception handling to catch both RuntimeError and ValueError for fallback. It also removes four pairs of test waiver entries for TestExaone4_5_33B::test_auto_dtype across multiple hardware configurations.

Changes

Qwen2.5-VL RoPE Fix

Layer / File(s) Summary
FlashInfer RoPE cache dtype and fallback
tensorrt_llm/_torch/models/modeling_qwen2vl.py
cos_sin_cache is now explicitly cast to float32 (in addition to being made contiguous), and the exception handler catches both RuntimeError and ValueError to trigger fallback to PyTorch RotaryEmbedding.apply_rotary_pos_emb.

Test Waiver Cleanup

Layer / File(s) Summary
Remove Exaone4_5_33B multimodal waivers
tests/integration/test_lists/waives.txt
Removes forced_chunked_prefill and full_budget waiver entries for TestExaone4_5_33B::test_auto_dtype on full:B300, full:GB200, full:GB300, and full:H20 configurations.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • NVIDIA/TensorRT-LLM#16030: Both PRs adjust the Exaone4_5_33B::test_auto_dtype waiver entries in the H20/multimodal area of waives.txt.
  • NVIDIA/TensorRT-LLM#16034: Both PRs edit waives.txt for the same TestExaone4_5_33B::test_auto_dtype forced_chunked_prefill/full_budget entries, one removing and the other adding related waivers.
  • NVIDIA/TensorRT-LLM#15959: Both PRs remove waive entries from waives.txt for CI/integration test cases.

Suggested reviewers: yechank-nvidia, achartier, jieli-matrix, nv-guomingz

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise, specific, and matches the main fix: casting cos_sin_cache to float32 for FlashInfer.
Description check ✅ Passed The description covers the bug, fix, tests, and link, though it does not follow the template headings exactly.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@yechank-nvidia yechank-nvidia enabled auto-merge (squash) July 7, 2026 12:54
@yechank-nvidia

Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #58004 [ run ] triggered by Bot. Commit: e413551 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #58004 [ run ] completed with state SUCCESS. Commit: e413551
/LLM/main/L0_MergeRequest_PR pipeline #46674 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

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.

3 participants