Skip to content

Conversation

dongfengy
Copy link
Collaborator

@dongfengy dongfengy commented Aug 7, 2025

Tested against test_w4_multigpu[{tp,ep,dp} * {2, 4}-TRITON]

Summary by CodeRabbit

  • New Features

    • Added support for padding and slicing of weights, scales, and activations to improve compatibility with tensor parallelism and specific GPU hardware.
    • Enhanced handling of quantized MoE weights for more flexible and robust multi-GPU setups.
  • Bug Fixes

    • Improved swizzling logic to better support NVIDIA H20 GPUs and removed previous shape restrictions.
  • Tests

    • Expanded integration and unit test coverage for multi-GPU scenarios and additional hidden/intermediate size combinations.
    • Renamed and updated test cases for clarity and broader parameterization.

Description

Test Coverage

GitHub Bot Help

/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...

Provide a user friendly way for developers to interact with a Jenkins server.

Run /bot [-h|--help] to print this help message.

See details below for each supported subcommand.

run [--reuse-test (optional)pipeline-id --disable-fail-fast --skip-test --stage-list "A10-PyTorch-1, xxx" --gpu-type "A30, H100_PCIe" --test-backend "pytorch, cpp" --add-multi-gpu-test --only-multi-gpu-test --disable-multi-gpu-test --post-merge --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" --detailed-log --debug(experimental)]

Launch build/test pipelines. All previously running jobs will be killed.

--reuse-test (optional)pipeline-id (OPTIONAL) : Allow the new pipeline to reuse build artifacts and skip successful test stages from a specified pipeline or the last pipeline if no pipeline-id is indicated. If the Git commit ID has changed, this option will be always ignored. The DEFAULT behavior of the bot is to reuse build artifacts and successful test results from the last pipeline.

--disable-reuse-test (OPTIONAL) : Explicitly prevent the pipeline from reusing build artifacts and skipping successful test stages from a previous pipeline. Ensure that all builds and tests are run regardless of previous successes.

--disable-fail-fast (OPTIONAL) : Disable fail fast on build/tests/infra failures.

--skip-test (OPTIONAL) : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does NOT update GitHub check status.

--stage-list "A10-PyTorch-1, xxx" (OPTIONAL) : Only run the specified test stages. Examples: "A10-PyTorch-1, xxx". Note: Does NOT update GitHub check status.

--gpu-type "A30, H100_PCIe" (OPTIONAL) : Only run the test stages on the specified GPU types. Examples: "A30, H100_PCIe". Note: Does NOT update GitHub check status.

--test-backend "pytorch, cpp" (OPTIONAL) : Skip test stages which don't match the specified backends. Only support [pytorch, cpp, tensorrt, triton]. Examples: "pytorch, cpp" (does not run test stages with tensorrt or triton backend). Note: Does NOT update GitHub pipeline status.

--only-multi-gpu-test (OPTIONAL) : Only run the multi-GPU tests. Note: Does NOT update GitHub check status.

--disable-multi-gpu-test (OPTIONAL) : Disable the multi-GPU tests. Note: Does NOT update GitHub check status.

--add-multi-gpu-test (OPTIONAL) : Force run the multi-GPU tests in addition to running L0 pre-merge pipeline.

--post-merge (OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.

--extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" (OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Examples: --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx".

--detailed-log (OPTIONAL) : Enable flushing out all logs to the Jenkins console. This will significantly increase the log volume and may slow down the job.

--debug (OPTIONAL) : Experimental feature. Enable access to the CI container for debugging purpose. Note: Specify exactly one stage in the stage-list parameter to access the appropriate container environment. Note: Does NOT update GitHub check status.

For guidance on mapping tests to stage names, see docs/source/reference/ci-overview.md
and the scripts/test_to_stage_mapping.py helper.

kill

kill

Kill all running builds associated with pull request.

skip

skip --comment COMMENT

Skip testing for latest commit on pull request. --comment "Reason for skipping build/test" is required. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.

reuse-pipeline

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.

@dongfengy dongfengy requested a review from a team as a code owner August 7, 2025 21:04
@dongfengy dongfengy requested a review from yuxianq August 7, 2025 21:04
Copy link
Contributor

coderabbitai bot commented Aug 7, 2025

📝 Walkthrough

Walkthrough

The changes introduce enhanced support for tensor parallelism and hardware-specific constraints in the MXFP4 Fused MoE module, including padding and slicing of weights, scales, and activations. Test coverage is expanded to new tensor shapes and parallel configurations, and test parameterization is updated to reflect these capabilities.

Changes

Cohort / File(s) Change Summary
Fused MoE Triton MXFP4 Implementation
tensorrt_llm/_torch/modules/fused_moe/fused_moe_triton.py
Added input/output dimension padding and tensor-parallel slicing for weights, scales, and activations. Adjusted swizzling logic for hardware compatibility. Introduced new attributes and helper functions for padding and TP offsets. Refactored and generalized shape handling throughout.
Integration Test Parameterization
tests/integration/defs/accuracy/test_llm_api_pytorch.py
Renamed test method and expanded parameterization to cover additional tensor parallel and expert parallel configurations. Removed a skip condition for certain test cases.
Unit Test Coverage Expansion
tests/unittest/_torch/modules/test_fused_moe.py
Updated test parameterization: removed a small shape case and added multiple new (hidden_size, intermediate_size) pairs for broader test coverage.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Module
    participant TritonMXFP4FusedMoEMethod
    participant GPU

    User->>Module: Calls apply(x, router_logits)
    Module->>TritonMXFP4FusedMoEMethod: apply(x, router_logits)
    TritonMXFP4FusedMoEMethod->>TritonMXFP4FusedMoEMethod: _maybe_pad_activation(x, in_dim_padding_offset)
    TritonMXFP4FusedMoEMethod->>GPU: GEMM 1 (using padded x and weights)
    TritonMXFP4FusedMoEMethod->>TritonMXFP4FusedMoEMethod: _maybe_pad_activation(intermediate, out_dim_padding_offset)
    TritonMXFP4FusedMoEMethod->>GPU: GEMM 2 (using padded intermediate and weights)
    TritonMXFP4FusedMoEMethod->>Module: Returns output
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Suggested reviewers

  • hlu1
  • Tracin

Note

🔌 MCP (Model Context Protocol) integration is now available in Early Access!

Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@dongfengy dongfengy requested a review from hlu1 August 7, 2025 21:04
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
tensorrt_llm/_torch/modules/fused_moe/fused_moe_triton.py (1)

1012-1056: Good implementation of padding logic for hardware constraints.

The _maybe_pad_weight_and_scale function correctly handles both Hopper-specific out-dimension padding and TP-related in-dimension padding with proper offset management. The assertions provide good validation.

Consider adding a docstring to document the padding requirements and the purpose of in_dim_padding_offset.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between db8dc97 and 17155b1.

📒 Files selected for processing (3)
  • tensorrt_llm/_torch/modules/fused_moe/fused_moe_triton.py (11 hunks)
  • tests/integration/defs/accuracy/test_llm_api_pytorch.py (1 hunks)
  • tests/unittest/_torch/modules/test_fused_moe.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

📄 CodeRabbit Inference Engine (CODING_GUIDELINES.md)

**/*.py: Python code should conform to Python 3.8+.
Indent Python code with 4 spaces. Do not use tabs.
Always maintain the namespace when importing in Python, even if only one class or function from a module is used.
Python filenames should use snake_case (e.g., some_file.py).
Python classes should use PascalCase (e.g., class SomeClass).
Python functions and methods should use snake_case (e.g., def my_awesome_function():).
Python local variables should use snake_case. Prefix k for variable names that start with a number (e.g., k_99th_percentile).
Python global variables should use upper snake_case and prefix G (e.g., G_MY_GLOBAL).
Python constants should use upper snake_case (e.g., MY_CONSTANT).
Avoid shadowing variables declared in an outer scope in Python.
Initialize all externally visible members of a Python class in the constructor.
For interfaces that may be used outside a Python file, prefer docstrings over comments.
Comments in Python should be reserved for code within a function, or interfaces that are local to a file.
Use Google style docstrings for Python classes and functions, which can be parsed by Sphinx.
Attributes and variables in Python can be documented inline; attribute docstrings will be rendered under the class docstring.
Avoid using reflection in Python when functionality can be easily achieved without it.
When using try-except blocks in Python, limit the except to the smallest set of errors possible.
When using try-except blocks to handle multiple possible variable types in Python, keep the body of the try as small as possible, using the else block to implement the logic.

Files:

  • tests/unittest/_torch/modules/test_fused_moe.py
  • tests/integration/defs/accuracy/test_llm_api_pytorch.py
  • tensorrt_llm/_torch/modules/fused_moe/fused_moe_triton.py
**/*.{cpp,h,hpp,cc,cxx,cu,py}

📄 CodeRabbit Inference Engine (CODING_GUIDELINES.md)

All TensorRT-LLM Open Source Software code should contain an NVIDIA copyright header that includes the current year. This includes .cpp, .h, .cu, .py, and any other source files which are compiled or interpreted.

Files:

  • tests/unittest/_torch/modules/test_fused_moe.py
  • tests/integration/defs/accuracy/test_llm_api_pytorch.py
  • tensorrt_llm/_torch/modules/fused_moe/fused_moe_triton.py
🧠 Learnings (1)
📚 Learning: 2025-07-28T17:06:08.621Z
Learnt from: moraxu
PR: NVIDIA/TensorRT-LLM#6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.

Applied to files:

  • tests/integration/defs/accuracy/test_llm_api_pytorch.py
🪛 Ruff (0.12.2)
tensorrt_llm/_torch/modules/fused_moe/fused_moe_triton.py

1091-1091: Line too long (141 > 120)

(E501)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Pre-commit Check
🔇 Additional comments (8)
tests/unittest/_torch/modules/test_fused_moe.py (1)

1301-1305: LGTM! Good expansion of test coverage for tensor parallelism.

The new test parameters with hidden_size=2880 and varying intermediate sizes are well-chosen to validate the enhanced TP support and padding/slicing logic in the Triton MoE implementation.

tests/integration/defs/accuracy/test_llm_api_pytorch.py (1)

2469-2474: LGTM! Good addition of TP=2 test configurations.

The rename to test_w4_multigpu better reflects the expanded scope of the test, and the addition of TP=2 configurations properly validates the PR's tensor parallelism improvements for Triton MOE.

tensorrt_llm/_torch/modules/fused_moe/fused_moe_triton.py (6)

629-654: LGTM! Good hardware compatibility handling for NVIDIA H20.

The addition of shape validation and the conditional use of StridedLayout for H20 GPUs is a good workaround for hardware-specific issues. The assertions will help catch shape mismatches early.


674-676: LGTM! Appropriate padding multiples for GPU memory alignment.

The padding multiples of 128 for input dimension and 256 for output dimension align with GPU memory access patterns and are necessary for proper hardware utilization.


687-708: LGTM! Improved scale tensor handling for tensor parallelism.

The change to use full-size scale tensors with ceil_div for proper block size handling is cleaner and correctly handles cases where dimensions aren't perfectly divisible by the block size (32). The deferred slicing approach is more maintainable.


1059-1096: LGTM! Robust tensor parallel slicing for scales.

The scale slicing logic correctly handles partial blocks when TP boundaries don't align with mxfp4's 32-element blocks. The w2_tp_offset tracking for partial block handling is a clever solution to maintain correct alignment.


1168-1179: LGTM! Consistent activation padding for tensor parallelism.

The _maybe_pad_activation function correctly applies the same padding logic to activations as used for weights, ensuring proper alignment for matrix multiplication operations.


1193-1193: LGTM! Correct application of activation padding.

The activation padding is properly applied before each GEMM operation, with appropriate offset handling for the second GEMM to account for TP partitioning.

Also applies to: 1248-1248

@dongfengy dongfengy self-assigned this Aug 11, 2025
@dongfengy dongfengy force-pushed the user/dongfengy/triton_tp_github branch from 17155b1 to ebcffd8 Compare August 14, 2025 21:30
@dongfengy
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #15345 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #15345 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #11578 completed with status: 'FAILURE'

Signed-off-by: Dongfeng Yu <[email protected]>

Add draft

Signed-off-by: Dongfeng Yu <[email protected]>

Fix

Signed-off-by: Dongfeng Yu <[email protected]>

Fix

Signed-off-by: Dongfeng Yu <[email protected]>

Fix

Signed-off-by: Dongfeng Yu <[email protected]>

Fix

Signed-off-by: Dongfeng Yu <[email protected]>
Signed-off-by: Dongfeng Yu <[email protected]>
Signed-off-by: Dongfeng Yu <[email protected]>
@dongfengy dongfengy force-pushed the user/dongfengy/triton_tp_github branch from ebcffd8 to 86670a8 Compare August 15, 2025 16:44
@dongfengy
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #15460 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #15460 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #11646 completed with status: 'FAILURE'

@dongfengy
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #15476 [ run ] triggered by Bot

@hlu1
Copy link
Collaborator

hlu1 commented Aug 15, 2025

/bot skip --comment "The failure is not related to the changes."

@hlu1 hlu1 enabled auto-merge (squash) August 15, 2025 20:34
@tensorrt-cicd
Copy link
Collaborator

PR_Github #15483 [ skip ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #15476 [ run ] completed with state ABORTED

@tensorrt-cicd
Copy link
Collaborator

PR_Github #15483 [ skip ] completed with state SUCCESS
Skipping testing for commit 86670a8

@hlu1 hlu1 merged commit 0ad0b96 into NVIDIA:main Aug 15, 2025
4 checks passed
dominicshanshan pushed a commit to dominicshanshan/TensorRT-LLM that referenced this pull request Aug 17, 2025
dominicshanshan pushed a commit to dominicshanshan/TensorRT-LLM that referenced this pull request Aug 17, 2025
dominicshanshan pushed a commit to dominicshanshan/TensorRT-LLM that referenced this pull request Aug 17, 2025
dominicshanshan pushed a commit to dominicshanshan/TensorRT-LLM that referenced this pull request Aug 17, 2025
dominicshanshan pushed a commit to dominicshanshan/TensorRT-LLM that referenced this pull request Aug 18, 2025
dominicshanshan pushed a commit to dominicshanshan/TensorRT-LLM that referenced this pull request Aug 18, 2025
dominicshanshan pushed a commit to dominicshanshan/TensorRT-LLM that referenced this pull request Aug 18, 2025
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