Skip to content

[TRTLLM-14609][chore] Remove legacy MoE path in DenseGEMMFusedMoE - #16865

Merged
xxi-nv merged 2 commits into
NVIDIA:mainfrom
xxi-nv:xxi/moe-rm-legacy-densegemm
Jul 29, 2026
Merged

[TRTLLM-14609][chore] Remove legacy MoE path in DenseGEMMFusedMoE#16865
xxi-nv merged 2 commits into
NVIDIA:mainfrom
xxi-nv:xxi/moe-rm-legacy-densegemm

Conversation

@xxi-nv

@xxi-nv xxi-nv commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Description

For the sake of MoE code health, this PR removes the legacy (pre-ConfigurableMoE) forward path from DenseGEMMFusedMoE.

ConfigurableMoE has been the default MoE path for months (ENABLE_CONFIGURABLE_MOE defaults to "1"), so the backend-internal forward pipeline is no longer reachable in the default flow.

What is removed (tensorrt_llm/_torch/modules/fused_moe/fused_moe_densegemm.py)

  • forward_chunk()
  • forward_impl()

Net effect: 75 deletions in a single file.

What is deliberately preserved

  • The live contract called by ConfigurableMoE / MoEScheduler is untouched: run_moe() and supports_moe_output_in_alltoall_workspace().

Test Coverage

Verified on OCI GB200 (1 node, 4 GPUs) with the container pinned by jenkins/current_image_tags.properties, running tests/unittest/_torch/modules/moe/test_moe_module.py filtered to this backend:

Scope Result
test_configurable_moe_single_gpu 5 passed
test_configurable_moe_multi_gpu 8 skipped (pre-existing suite skips, see note)
test_configurable_moe_multi_gpu_eplb no matching cases for this backend

5 passed, 0 failed.

Note on multi-GPU coverage. All 8 collected multi-GPU cases for DENSEGEMM (parallel=DTP/TTP x comm=NVLINK_ONE_SIDED/TWO_SIDED x quant=NVFP4) are skipped by the existing skip logic in tests/unittest/_torch/modules/moe/moe_test_utils.py. That file is not touched by this PR, so these cases skip identically on unmodified main — the skips are pre-existing and not caused by this change. The two removed methods are not referenced by ConfigurableMoE or MoEScheduler, so the residual risk is low, but reviewers should be aware that multi-GPU coverage for this backend comes from the unchanged shared scheduler path rather than from backend-specific cases.

PR Checklist

  • PR title follows [JIRA/NVBUG][type] description
  • Commit is signed off (DCO)
  • Tests pass for the affected backend

Dev Engineer Review

  • Removed the unused legacy forward_chunk() and forward_impl() methods, along with the obsolete allgather import, from DenseGEMMFusedMoE.
  • The active ConfigurableMoE interface remains unchanged, including run_moe() and supports_moe_output_in_alltoall_workspace().
  • No configuration or test-list files were modified.
  • Reported validation: 5 single-GPU backend tests pass; 8 multi-GPU cases remain skipped by existing test logic.

QA Engineer Review

No test changes.

ConfigurableMoE has been the default MoE path for months (ENABLE_CONFIGURABLE_MOE defaults to 1) and drives compute through backend.run_moe while the wrapper owns communication and chunk scheduling. The backend-level legacy MoE forward path is dead code on that path. Remove it; run_moe and the live ConfigurableMoE contract (quantize_input, get_workspaces, supports_moe_output_in_alltoall_workspace, forward_fake, etc.) are unchanged.

Signed-off-by: xxi <xxi@nvidia.com>
@xxi-nv

xxi-nv commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run

@xxi-nv
xxi-nv marked this pull request as ready for review July 26, 2026 23:58
@xxi-nv
xxi-nv requested a review from a team as a code owner July 26, 2026 23:58
@xxi-nv
xxi-nv requested review from leslie-fang25 and rosong11 July 26, 2026 23:59
@coderabbitai

coderabbitai Bot commented Jul 26, 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: 5096415b-a82e-4096-9e7b-fe746f2985c3

📥 Commits

Reviewing files that changed from the base of the PR and between 4b7d719 and 43cee82.

📒 Files selected for processing (1)
  • tensorrt_llm/_torch/modules/fused_moe/fused_moe_densegemm.py
💤 Files with no reviewable changes (1)
  • tensorrt_llm/_torch/modules/fused_moe/fused_moe_densegemm.py

Walkthrough

The DenseGEMMFusedMoE implementation removes the unused allgather import and deletes the legacy forward_chunk and forward_impl methods, leaving the existing run_moe path intact.

Changes

DenseGEMM Fused MoE cleanup

Layer / File(s) Summary
Remove legacy forward paths
tensorrt_llm/_torch/modules/fused_moe/fused_moe_densegemm.py
Removes the unused import and deletes forward_chunk and forward_impl, including their routing, quantization, distributed communication, MoE execution, and reduction logic.

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

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows the required ticket-and-type format and accurately summarizes removal of the legacy MoE path.
Description check ✅ Passed The description includes the issue, solution, test coverage, and checklist, so it is mostly complete and on-template.
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.

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #61760 [ run ] triggered by Bot. Commit: 43cee82 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #61760 [ run ] completed with state SUCCESS. Commit: 43cee82
/LLM/main/L0_MergeRequest_PR pipeline #49962 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

@xxi-nv

xxi-nv commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #61820 [ run ] triggered by Bot. Commit: 43cee82 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #61820 [ run ] completed with state FAILURE. Commit: 43cee82
/LLM/main/L0_MergeRequest_PR pipeline #50013 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

@xxi-nv

xxi-nv commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #62194 [ run ] triggered by Bot. Commit: 0fc1bb0 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #62194 [ run ] completed with state FAILURE. Commit: 0fc1bb0
/LLM/main/L0_MergeRequest_PR pipeline #50365 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

@xxi-nv

xxi-nv commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #62264 [ run ] triggered by Bot. Commit: 0fc1bb0 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #62264 [ run ] completed with state FAILURE. Commit: 0fc1bb0
/LLM/main/L0_MergeRequest_PR pipeline #50433 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

@xxi-nv

xxi-nv commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #62314 [ run ] triggered by Bot. Commit: 0fc1bb0 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #62314 [ run ] completed with state FAILURE. Commit: 0fc1bb0
/LLM/main/L0_MergeRequest_PR pipeline #50484 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

@xxi-nv

xxi-nv commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #62342 [ run ] triggered by Bot. Commit: 0fc1bb0 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #62342 [ run ] completed with state SUCCESS. Commit: 0fc1bb0
/LLM/main/L0_MergeRequest_PR pipeline #50510 completed with status: 'SUCCESS'
Pipeline passed with automatic retried tests. Check the rerun report for details.

CI Report

Link to invocation

@xxi-nv
xxi-nv merged commit c4afe88 into NVIDIA:main Jul 29, 2026
7 checks passed
@xxi-nv
xxi-nv deleted the xxi/moe-rm-legacy-densegemm branch July 29, 2026 02:47
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