[https://nvbugs/6644644][fix] Replay both verified orphan payloads onto current origin/main as a single… - #18142
Conversation
…used Two layers of one optional-dependency defect, landed together because they are the same bug at two sites and the failing frame must stay visible in this commit's own scope. Layer 1 -- tensorrt_llm/_torch/modules/fused_moe/fused_moe_cute_dsl.py cute_dsl_custom_ops' `if IS_CUTLASS_DSL_AVAILABLE:` block runs to end of file with no else-branch, so everything it defines is simply absent when the optional cutlass DSL is not installed. This module imported four Sm100BlockScaledContiguous*Runner names from it at module scope. Only GroupedGemmInputsHelper, also in that import, is top-level; the other four are guard-only. Because create_moe imports this module eagerly and it sits under _torch.models, the resulting ImportError propagated through fused_moe/__init__.py, modeling_utils.py, _torch/models/__init__.py and registry.create_input_processor to llm.py's _build_model -- so on a cutlass-less Blackwell host every LLM() construction died at import, well past the CuTe backends. That is what failed this test: it builds a multimodal encoder LLM and never reached its own assertions. The four names are needed at exactly one place, the isinstance check in runner_tactic_comb_checker, and they are flat TunableRunner leaves with no subclasses, no __all__ and no other reference in the tree. Reaching that line means a CuteDslFusedMoENvfp4Runner is already under autotune, so the DSL is necessarily installed. Import them there instead of at module scope, matching how cute_dsl_mla.py and dsa/metadata.py already reach into this guard from modules that must stay importable. This adds no module-scope binding, no fallback branch and no new runtime state, so there is no reachable path where the feature is quietly off: with the DSL present the runners resolve and the isinstance tuple is unchanged; without it those symbols never existed, and the crash is what is removed, not a capability. The provider side was ruled out mechanically rather than by preference -- cute_dsl_custom_ops.py fails confidentiality-scan on unmodified origin/main via pre-existing upstream prose, so no commit touching it can ship. Layer 2 -- tests/unittest/_torch/thop/parallel/test_cute_dsl_moe.py Fixing layer 1 makes this module collectable for the first time, which exposes a second missing guard: four nvfp4 grouped-GEMM tests call torch.ops.trtllm.cute_dsl_nvfp4_* ops registered only inside the same availability block, but carried only an SM-version skip. On an SM100 host without the DSL that skip passes, so they ran to the op call and died with AttributeError on the trtllm op namespace. Add the same IS_CUTLASS_DSL_AVAILABLE mark the other cute-DSL test modules already use (test_fp4_linear.py, test_indexer_topk.py). Coverage strictly expands, so this is not a waiver: with the DSL absent the file goes from 0 tests collected (collection error) to 420 run and 360 skipped; with the DSL present it goes from 0 to all 780 run. Nothing that previously executed stops executing. Verified on B200: target test 1 passed, and the full module now exits 0 with 420 passed / 360 skipped, the skips printing as [72] [72] [72] [144]. Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. WalkthroughThe fused MoE module conditionally imports CuteDSL runners and checks tile-size tactics only when CuteDSL is available. Related Blackwell NVFP4 tests skip when the dependency is unavailable. ChangesCuteDSL availability handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The localized import and test-skip changes leave no actionable merge-blocking risk at the current head; the PR is merge-ready after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the root cause, fix, affected tests, test plan, and linked bug. It omits the template headings and checklist, but it contains the critical information needed to understand the change. Full details: Title checkExplanation The title uses the required NVBugs and fix format and refers to the commit replay described in the objectives. It does not clearly state the primary code fix, but it remains related to the pull request.
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
It looks it was duplicating with #15986 |
|
/bot run |
|
PR_Github #69337 [ run ] triggered by Bot. Commit: |
|
PR_Github #69337 [ run ] completed with state
|
|
/bot run |
|
PR_Github #69466 [ run ] triggered by Bot. Commit: |
|
PR_Github #69466 [ run ] completed with state
|
|
/bot run |
|
PR_Github #69516 [ run ] triggered by Bot. Commit: |
|
PR_Github #69516 [ run ] completed with state
|
Signed-off-by: William Zhang <133824995+2ez4bz@users.noreply.github.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/bot help |
GitHub Bot Help
Provide a user friendly way for developers to interact with a Jenkins server. Run See details below for each supported subcommand. Details
Launch build/test pipelines. All previously running jobs will be killed.
kill
Kill all running builds associated with pull request. skip
Skip testing for latest commit on pull request. reuse-pipeline
Reuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break. |
|
/bot run --add-multi-gpu-test |
|
PR_Github #69875 [ run ] triggered by Bot. Commit: |
|
PR_Github #69875 [ run ] completed with state
|
Signed-off-by: William Zhang <133824995+2ez4bz@users.noreply.github.com>
|
/bot run |
|
/bot run --add-multi-gpu-test |
|
PR_Github #69916 [ run ] triggered by Bot. Commit: |
|
PR_Github #69918 [ run ] triggered by Bot. Commit: |
|
PR_Github #69916 [ run ] completed with state |
|
PR_Github #69918 [ run ] completed with state
|
|
/bot run --add-multi-gpu-test |
|
PR_Github #69994 [ run ] triggered by Bot. Commit: |
|
PR_Github #69994 [ run ] completed with state
|
|
/bot run --add-multi-gpu-test |
|
PR_Github #70037 [ run ] triggered by Bot. Commit: |
|
PR_Github #70037 [ run ] completed with state |
Summary
Sm100BlockScaledContiguous*Runnernames are imported at module scope but defined only insidecute_dsl_custom_ops' else-lessif IS_CUTLASS_DSL_AVAILABLE:block, so on a cutlass-less host the ImportError propagates via create_moe → _torch/models → create_input_processor and kills everyLLM()construction; fixing that unmasks four tests calling DSL-only ops with no availability mark.IS_CUTLASS_DSL_AVAILABLEskipif the sibling cute-DSL test modules already carry.pytest "tests/unittest/_torch/multimodal/test_mm_encoder_standalone.py::test_kv_event_mm_keys_with_uuid[False-hex]" -vTest plan
Links
Dev Engineer Review
Sm100BlockScaledContiguous*Runnerimports withIS_CUTLASS_DSL_AVAILABLE._TILE_SIZE_CHECKED_RUNNERSempty when Cutlass DSL is unavailable.QA Engineer Review
tests/unittest/_torch/thop/parallel/test_cute_dsl_moe.py.tests/integration/test_lists/.