Skip to content

Conversation

Yuening-wa
Copy link
Collaborator

@Yuening-wa Yuening-wa commented Aug 5, 2025

Summary by CodeRabbit

  • New Features

    • Added support for weight-only quantization modes, including per-channel and group scaling variants, for Mixture of Experts (MoE) layers.
    • Introduced new quantization method handling and configuration options in MoE modules and custom operators.
    • Enhanced test coverage with new tests for weight-only quantized MoE operations, including per-channel and groupwise modes.
  • Bug Fixes

    • Refined error handling for INT8-GPTQ support to only trigger under specific conditions.
  • Tests

    • Added and updated tests to validate correctness of weight-only quantized MoE implementations and tolerance calculations.
    • Improved test utilities for consistent quantization tolerance computation.
  • Refactor

    • Updated and renamed test classes and methods for clarity and expanded quantization mode support.

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.

@Yuening-wa Yuening-wa requested a review from a team as a code owner August 5, 2025 09:54
Copy link
Contributor

coderabbitai bot commented Aug 5, 2025

📝 Walkthrough

Walkthrough

This update extends weight-only quantization support for Mixture-of-Experts (MoE) layers, adding per-channel and group scaling variants. It introduces new quantization method classes, updates the MoE runner and custom operator interfaces, adjusts tensor shape and quantization parameter handling, and adds comprehensive tests for the new quantization modes.

Changes

Cohort / File(s) Change Summary
C++ MoE kernel and runner logic
cpp/tensorrt_llm/thop/moeOp.cpp
Adds per-channel and group scaling weight-only quantization support in FusedMoeRunner; updates constructor, quantization logic, dimension checks, quant param extraction, and Torch bindings.
PyTorch custom ops and interface
tensorrt_llm/_torch/custom_ops/torch_custom_ops.py
Adds use_woq_per_channel and use_woq_group_scaling parameters to MoERunner and fused_moe; updates interface and fake implementation for quantization shape logic.
Cutlass MoE module logic
tensorrt_llm/_torch/modules/fused_moe/fused_moe_cutlass.py
Integrates weight-only quantization mode, adds detection logic and flags, updates quantization method selection, and passes new flags to the custom operator.
Quantization method implementation
tensorrt_llm/_torch/modules/fused_moe/quantization.py
Adds WeightOnlyFusedMoEMethod class and scale tuple, implements weight-only quantization logic, and updates weight loading and scale handling.
MoE layer constructor logic
tensorrt_llm/layers/moe.py
Narrows the condition for raising NotImplementedError for INT8-GPTQ support to cases where group_size > 0.
Test helper for quantization tolerance
tests/unittest/_torch/helpers.py
Adds calc_woq_tolerence, a helper to compute absolute tolerance for weight-only quantized outputs.
Fused MoE unit test (PyTorch)
tests/unittest/_torch/modules/test_fused_moe.py
Adds test_fused_moe_weight_only to validate fused MoE with int8 weight-only quantization using reference computation and tolerance checks.
Quantized GEMM test helper update
tests/unittest/_torch/thop/test_weight_only_quant_gemm.py
Replaces local tolerance calculation with imported calc_woq_tolerence helper.
TensorRT MoE quantization tests
tests/unittest/trt/quantization/test_moe_weight_only_quant_matmul.py
Renames class and methods for clarity, adds per-channel quantization test, updates quant mode handling, and integrates new test logic for int8/groupwise modes.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant PyTorchModule
    participant FusedMoERunner
    participant CutlassMoERunner
    participant QuantizationMethod

    User->>PyTorchModule: Calls fused_moe(..., use_woq_per_channel, use_woq_group_scaling)
    PyTorchModule->>FusedMoERunner: Instantiates with quantization flags
    FusedMoERunner->>CutlassMoERunner: create_weight_quant_runner() based on flags
    FusedMoERunner->>QuantizationMethod: Extracts quant scales and parameters
    FusedMoERunner->>CutlassMoERunner: Runs kernel with quantization config
    CutlassMoERunner-->>FusedMoERunner: Returns result
    FusedMoERunner-->>PyTorchModule: Returns output
    PyTorchModule-->>User: Output tensor(s)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Suggested labels

Community want to contribute

Suggested reviewers

  • shaharmor98
  • litaotju
  • nv-guomingz
  • symphonylyh
  • suyoggupta

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 90730fc and 269658b.

📒 Files selected for processing (1)
  • cpp/tensorrt_llm/thop/moeOp.cpp (9 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
⏰ 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 (9)
cpp/tensorrt_llm/thop/moeOp.cpp (9)

96-118: LGTM! Well-structured template method for weight quantization runners.

The method correctly creates the appropriate runner based on quantization type, with proper conditional compilation guards for FP8 support and appropriate error handling for unsupported types.


121-122: LGTM! Constructor properly extended for weight-only quantization parameters.

The new parameters use_woq_per_channel and use_woq_group_scaling are correctly added and stored in member variables following the existing pattern.

Also applies to: 129-130


180-192: LGTM! Proper weight-only quantization runner instantiation.

The logic correctly handles different weight-only quantization modes, sets the appropriate inner dimension multiplier for INT4, and creates the correct runner based on activation type with proper error handling.


277-296: LGTM! Proper dimension handling for per-channel quantization.

The conditional logic correctly handles different tensor layouts for per-channel vs other quantization modes, with appropriate dimension validation between fc1 and fc2 expert weights.


292-296: LGTM! Consistent dimension calculation for per-channel quantization.

The dimension calculations properly handle the different tensor layouts for per-channel quantization, with consistent implementation across both runMoe and runGemmProfile methods.

Also applies to: 522-526


528-528: LGTM! Consistent group size definition addresses previous feedback.

The addition of WEIGHT_QUANT_GROUP_SIZE constant ensures consistency across the codebase and properly addresses the past review comment about inconsistent group size definitions.

Also applies to: 609-610, 919-919


897-933: LGTM! Comprehensive quantization parameter handling.

The logic properly handles different weight-only quantization modes with appropriate parameter extraction and validation. The distinction between per-channel and group scaling modes is correctly implemented with proper error handling for unsupported combinations.


953-971: LGTM! Clean helper methods for quantization type checking.

The new helper methods isWInt8Quant() and isWeightOnlyQuant() follow the existing pattern and provide clean abstractions for checking different weight-only quantization types.


991-991: LGTM! Torch binding correctly updated for new constructor parameters.

The binding properly includes all constructor parameters including the new weight-only quantization flags, maintaining consistency with the C++ constructor signature.

✨ 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.

@Yuening-wa Yuening-wa force-pushed the user/yueningl/moe_int8_weight_only_quant_support branch 2 times, most recently from b9b6ffe to 107befa Compare August 5, 2025 10:13
@Yuening-wa
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #14138 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

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

@Yuening-wa
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #14211 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #14211 [ run ] completed with state FAILURE
/LLM/main/L0_MergeRequest_PR pipeline #10732 completed with status: 'FAILURE'

@Yuening-wa Yuening-wa force-pushed the user/yueningl/moe_int8_weight_only_quant_support branch from 107befa to c26bf90 Compare August 6, 2025 06:23
@Yuening-wa Yuening-wa requested review from a team as code owners August 6, 2025 06:23
@Yuening-wa Yuening-wa requested a review from yuxianq August 6, 2025 06:23
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: 6

🧹 Nitpick comments (3)
tests/unittest/_torch/helpers.py (1)

78-87: Fix typo in function name.

The function implementation is correct and follows proper quantization tolerance calculation principles. However, there's a spelling error in the function name.

Apply this diff to fix the typo:

-def calc_woq_tolerence(x: torch.Tensor, weight_dtype: torch.dtype):
+def calc_woq_tolerance(x: torch.Tensor, weight_dtype: torch.dtype):

Note: You'll also need to update the import statements in files that use this function to match the corrected name.

tensorrt_llm/_torch/modules/fused_moe/quantization.py (2)

942-951: Clean up commented INT4 code.

The commented INT4 code should be removed to improve code clarity. If INT4 support is planned, track it in a TODO comment instead.

         # preprocess the weights for mixed gemm
         if module.quant_config.layer_quant_mode.is_int8_weight_only():
             weight_dtype = torch.int8
-        # elif module.quant_config.layer_quant_mode.is_int4_weight_only():
-        #     weight_dtype = torch.quint4x2
-        #     packer = torch.ops.trtllm.pack_int8_tensor_to_packed_int4
-        #     unpacker = torch.ops.trtllm.unpack_int4_packed_tensor_to_int8
-        #     w31_weight_shard = packer(
-        #         unpacker(w31_weight_shard.cpu()).T.contiguous()).to(
-        #             w31_weight_shard.device)
+        # TODO: Add INT4 weight-only support in the future

972-981: Remove duplicate commented INT4 code.

Similar to the previous method, remove the commented INT4 code to maintain code cleanliness.

         # preprocess the weights for mixed gemm
         if module.quant_config.layer_quant_mode.is_int8_weight_only():
             weight_dtype = torch.int8
-        # elif module.quant_config.layer_quant_mode.is_int4_weight_only():
-        #     weight_dtype = torch.quint4x2
-        #     packer = torch.ops.trtllm.pack_int8_tensor_to_packed_int4
-        #     unpacker = torch.ops.trtllm.unpack_int4_packed_tensor_to_int8
-        #     w31_weight_shard = packer(
-        #         unpacker(w31_weight_shard.cpu()).T.contiguous()).to(
-        #             w31_weight_shard.device)
+        # TODO: Add INT4 weight-only support in the future
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 83ee91e and c26bf90.

📒 Files selected for processing (9)
  • cpp/tensorrt_llm/thop/moeOp.cpp (9 hunks)
  • tensorrt_llm/_torch/custom_ops/torch_custom_ops.py (5 hunks)
  • tensorrt_llm/_torch/modules/fused_moe/fused_moe_cutlass.py (8 hunks)
  • tensorrt_llm/_torch/modules/fused_moe/quantization.py (5 hunks)
  • tensorrt_llm/layers/moe.py (1 hunks)
  • tests/unittest/_torch/helpers.py (1 hunks)
  • tests/unittest/_torch/modules/test_fused_moe.py (2 hunks)
  • tests/unittest/_torch/thop/test_weight_only_quant_gemm.py (2 hunks)
  • tests/unittest/trt/quantization/test_moe_weight_only_quant_matmul.py (6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py

📄 CodeRabbit Inference Engine (CODING_GUIDELINES.md)

**/*.py: The code developed for TensorRT-LLM 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 file, prefer docstrings over comments in Python.
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, and attribute docstrings will be rendered under the docstring for the class.
Avoid using reflection in Python when functionality can be easily achieved without reflection.
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/helpers.py
  • tensorrt_llm/layers/moe.py
  • tests/unittest/trt/quantization/test_moe_weight_only_quant_matmul.py
  • tests/unittest/_torch/thop/test_weight_only_quant_gemm.py
  • tests/unittest/_torch/modules/test_fused_moe.py
  • tensorrt_llm/_torch/modules/fused_moe/fused_moe_cutlass.py
  • tensorrt_llm/_torch/custom_ops/torch_custom_ops.py
  • tensorrt_llm/_torch/modules/fused_moe/quantization.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/helpers.py
  • tensorrt_llm/layers/moe.py
  • tests/unittest/trt/quantization/test_moe_weight_only_quant_matmul.py
  • tests/unittest/_torch/thop/test_weight_only_quant_gemm.py
  • tests/unittest/_torch/modules/test_fused_moe.py
  • tensorrt_llm/_torch/modules/fused_moe/fused_moe_cutlass.py
  • tensorrt_llm/_torch/custom_ops/torch_custom_ops.py
  • cpp/tensorrt_llm/thop/moeOp.cpp
  • tensorrt_llm/_torch/modules/fused_moe/quantization.py
**/*.{cpp,h,hpp,cc,cxx}

📄 CodeRabbit Inference Engine (CODING_GUIDELINES.md)

**/*.{cpp,h,hpp,cc,cxx}: Closing braces of namespaces should have a comment saying the namespace it closes (e.g., } // namespace foo).
Prefer const or constexpr variables over #defines whenever possible, as the latter are not visible to the compiler.
A variable that is not modified after its initialization should be declared as const.
Except 0 (only used in comparison for checking signness/existence/emptiness) and nullptr, true, false, all other literals should only be used for variable initialization.
Use the Allman indentation style for braces in C++ code.
Put the semicolon for an empty for or while loop in a new line.
The statement forming the body of a switch, while, do .. while or for statement shall be a compound statement (use brace-delimited statements).
If and else should always be followed by brace-delimited statements, even if empty or a single statement.
C++ filenames should use camel case with first letter lowercase (e.g., thisIsAFilename.cpp), and all files involved in the compilation of a target must have filenames that are case-insensitive unique.
All types (including class names) are camel case with uppercase first letter (e.g., FooBarClass).
Local variables, methods, and namespaces use camel case with first letter lowercase (e.g., localFooBar).
Non-magic-number global variables that are non-static and not defined in anonymous namespace use camel case prefixed by a lower case 'g' (e.g., gDontUseGlobalFoos).
Non-magic-number global variables that are static or defined in an anonymous namespace use camel case prefixed by a lower case 's' (e.g., sMutableStaticGlobal).
Locally visible static variable uses camel case with lowercase prefix 's' as the first letter of the name (e.g., static std::once_flag sFlag;).
Class member variables use camelcase prefixed with an 'm' (e.g., mNbFooValues). Public member variables do not require the 'm' prefix but it is encouraged for clarity.
Enumerations, global constants, static constants at class-scope and function-...

Files:

  • cpp/tensorrt_llm/thop/moeOp.cpp
🧠 Learnings (4)
📓 Common learnings
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.
📚 Learning: in tensorrt-llm testing, it's common to have both cli flow tests (test_cli_flow.py) and pytorch api ...
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/unittest/trt/quantization/test_moe_weight_only_quant_matmul.py
  • tests/unittest/_torch/modules/test_fused_moe.py
  • tensorrt_llm/_torch/modules/fused_moe/quantization.py
📚 Learning: in tensorrt-llm's multimodal processing pipeline, shared tensor recovery using `from_shared_tensor()...
Learnt from: yechank-nvidia
PR: NVIDIA/TensorRT-LLM#6254
File: tensorrt_llm/_torch/pyexecutor/model_engine.py:1201-1204
Timestamp: 2025-07-22T09:22:14.726Z
Learning: In TensorRT-LLM's multimodal processing pipeline, shared tensor recovery using `from_shared_tensor()` is only needed during the context phase. Generation requests reuse the already-recovered tensor data and only need to call `strip_for_generation()` to remove unnecessary multimodal data while preserving the recovered tensors. This avoids redundant tensor recovery operations during generation.

Applied to files:

  • tensorrt_llm/_torch/modules/fused_moe/quantization.py
📚 Learning: in tensorrt-llm, examples directory can have different dependency versions than the root requirement...
Learnt from: yibinl-nvidia
PR: NVIDIA/TensorRT-LLM#6506
File: examples/models/core/mixtral/requirements.txt:3-3
Timestamp: 2025-08-01T15:14:45.673Z
Learning: In TensorRT-LLM, examples directory can have different dependency versions than the root requirements.txt file. Version conflicts between root and examples dependencies are acceptable because examples are designed to be standalone and self-contained.

Applied to files:

  • tensorrt_llm/_torch/modules/fused_moe/quantization.py
⏰ 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 (15)
tensorrt_llm/layers/moe.py (1)

771-772: LGTM! Logical refinement to enable INT8 weight-only quantization.

This change correctly narrows the error condition to only prevent INT8 weight quantization when group scaling is used (group_size > 0), while allowing INT8 weight-only quantization without group scaling to proceed. This aligns perfectly with the PR's objective of adding MoE INT8 weight-only quantization support.

tests/unittest/_torch/thop/test_weight_only_quant_gemm.py (2)

18-18: Good refactoring to use centralized tolerance calculation.

Importing the helper function promotes code reuse and ensures consistent tolerance calculations across weight-only quantization tests.

Note: When the typo in the helper function name is fixed, this import will need to be updated to calc_woq_tolerance.


70-71: Excellent refactoring to eliminate code duplication.

Replacing the local tolerance calculation with the centralized helper function is a good practice that ensures consistency across tests while reducing code duplication.

Note: When the helper function name is corrected to calc_woq_tolerance, this line will need to be updated accordingly.

tensorrt_llm/_torch/custom_ops/torch_custom_ops.py (2)

43-77: LGTM! Clean integration of weight-only quantization flags.

The new use_woq_per_channel and use_woq_group_scaling parameters are properly integrated into the MoERunner class, following the existing pattern for quantization flags.


146-147: LGTM! Correct handling of weight tensor shapes.

The fake implementation properly accounts for the different weight tensor layout when use_woq_per_channel is enabled, where the hidden dimension moves to index 2.

Also applies to: 184-185, 258-268

tensorrt_llm/_torch/modules/fused_moe/fused_moe_cutlass.py (3)

147-156: LGTM! Well-defined properties for weight-only quantization modes.

The properties correctly distinguish between per-channel and per-group weight-only quantization modes.


169-170: LGTM! Correct quantization method selection.

The code properly returns WeightOnlyFusedMoEMethod for per-channel weight-only quantization.


250-257: Verify the flag setting logic for group scaling.

The use_woq_group_scaling flag is set to True for W4A8 quantization (line 252) and potentially set again based on has_woq_per_group_scaling (line 257). Please verify this is intentional and consider adding a comment to clarify the precedence.

tests/unittest/trt/quantization/test_moe_weight_only_quant_matmul.py (2)

72-110: LGTM! Correct handling of per-channel vs group-wise quantization.

The code properly determines use_int8 from the quantization mode and correctly assigns either per_channel_scale or weights_scaling_factor based on the group size.


274-363: LGTM! Comprehensive test coverage for per-channel weight-only quantization.

The new _woq_moe_matmul_per_channel method and test_moe_w8a16 test provide thorough coverage for per-channel weight-only quantization, following the established testing patterns.

tensorrt_llm/_torch/modules/fused_moe/quantization.py (3)

9-10: Good refactoring: Moving import to module level.

Moving preprocess_weights_for_mixed_gemm to module-level import is appropriate since it's now used by multiple quantization method classes.


64-67: LGTM! Consistent structure for weight-only quantization scales.

The new namedtuple follows the established pattern and clearly defines the weight scale tensors needed for weight-only quantization.


869-869: Clarify the SM version override logic.

The code sets sm_version = 80 for architectures >= 90. This seems like a workaround that could lead to confusion. Please add a comment explaining why newer architectures are treated as SM 80 for weight-only quantization.

cpp/tensorrt_llm/thop/moeOp.cpp (2)

184-196: LGTM! Weight-only quantization initialization is well-structured.

The initialization logic for weight-only quantization correctly handles different activation types and sets the appropriate inner dimension multiplier for INT4.


281-300: Correct handling of different tensor layouts.

The dimension checks and size calculations properly handle both per-channel and standard weight-only quantization layouts.

@Yuening-wa Yuening-wa force-pushed the user/yueningl/moe_int8_weight_only_quant_support branch from e768d61 to 90730fc Compare August 6, 2025 07:39
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

♻️ Duplicate comments (2)
cpp/tensorrt_llm/thop/moeOp.cpp (2)

525-533: LGTM! Group size consistency issue resolved.

The dimension calculation logic correctly mirrors the main execution path and uses the new WEIGHT_QUANT_GROUP_SIZE constant, which addresses the previous review comment about inconsistent group size definitions across the codebase.


610-615: LGTM! New member variables and constant improve code organization.

The addition of the weight-only quantization flags and the WEIGHT_QUANT_GROUP_SIZE constant enhances code clarity and directly addresses the previous review comment about maintaining consistent group size definitions.

🧹 Nitpick comments (1)
cpp/tensorrt_llm/thop/moeOp.cpp (1)

96-123: LGTM with minor optimization opportunity.

The template method correctly handles different weight quantization types and addresses the previous duplicated runner instantiation issue. However, there's still a minor duplication that could be optimized.

Consider consolidating the FP8-enabled and FP8-disabled paths for the non-group-scaling case:

 #ifdef ENABLE_FP8
             if (mUseW4A8GroupScaling)
             {
                 return std::make_unique<
                     kernels::CutlassMoeFCRunner<__nv_fp8_e4m3, cutlass::uint4b_t, TypeAct, TypeAct>>();
             }
-            else
-            {
-                return std::make_unique<kernels::CutlassMoeFCRunner<TypeAct, cutlass::uint4b_t>>();
-            }
-#else
+#endif
             return std::make_unique<kernels::CutlassMoeFCRunner<TypeAct, cutlass::uint4b_t>>();
-#endif
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between c26bf90 and e768d61.

📒 Files selected for processing (3)
  • cpp/tensorrt_llm/thop/moeOp.cpp (9 hunks)
  • tensorrt_llm/_torch/modules/fused_moe/quantization.py (5 hunks)
  • tests/unittest/_torch/modules/test_fused_moe.py (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/unittest/_torch/modules/test_fused_moe.py
  • tensorrt_llm/_torch/modules/fused_moe/quantization.py
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
⏰ 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 (5)
cpp/tensorrt_llm/thop/moeOp.cpp (5)

125-197: LGTM! Constructor properly handles new quantization modes.

The constructor correctly:

  • Accepts the new weight-only quantization parameters
  • Initializes the corresponding member variables
  • Sets the appropriate inner dimension multiplier for Int4 quantization
  • Uses the new template method for weight quantization runner creation
  • Provides proper error handling for unsupported activation types

282-301: LGTM! Dimension calculations correctly handle per-channel quantization.

The conditional logic properly adjusts dimension checks and calculations based on the per-channel weight-only quantization mode:

  • Switches tensor dimension indices appropriately for different quantization layouts
  • Maintains consistency between fc1 and fc2 expert weight dimensions
  • Correctly recalculates hidden_size and inter_size for per-channel mode

902-939: LGTM! Comprehensive quantization parameter handling.

The method correctly handles different weight-only quantization modes:

  • Per-channel quantization with appropriate 2-scale validation
  • Int4 group scaling with comprehensive 8-parameter setup
  • Proper use of the consistent WEIGHT_QUANT_GROUP_SIZE constant
  • Good error handling for unsupported configurations

The parameter validation and construction logic is thorough and well-structured.


958-976: LGTM! Helper methods improve code clarity.

The new helper methods provide clean abstractions:

  • isWInt8Quant() clearly identifies int8 weight quantization
  • isWeightOnlyQuant() provides a unified check for weight-only quantization modes

These methods enhance readability and maintainability by encapsulating quantization type logic.


996-996: LGTM! Torch binding correctly updated for new parameters.

The constructor binding properly exposes the new weight-only quantization parameters (use_woq_per_channel and use_woq_group_scaling) to the Python interface, maintaining API consistency.

@Yuening-wa
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #14268 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #14268 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #10773 completed with status: 'SUCCESS'

@Yuening-wa Yuening-wa force-pushed the user/yueningl/moe_int8_weight_only_quant_support branch from 269658b to 455cb26 Compare August 9, 2025 15:53
@Yuening-wa
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #14680 [ run ] triggered by Bot

@Yuening-wa Yuening-wa force-pushed the user/yueningl/moe_int8_weight_only_quant_support branch from bd6f1ac to f1aa234 Compare August 14, 2025 04:44
@Yuening-wa
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #15223 [ run ] triggered by Bot

@WanchaoYao
Copy link

Do u have e2e test on some models?

@Yuening-wa
Copy link
Collaborator Author

Do u have e2e test on some models?

I am working on the e2e test and considering to open another PR for it.

@Yuening-wa Yuening-wa force-pushed the user/yueningl/moe_int8_weight_only_quant_support branch from f1aa234 to 45bc810 Compare August 15, 2025 01:59
@Yuening-wa
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #15368 [ run ] triggered by Bot

@yuxianq yuxianq enabled auto-merge (squash) August 15, 2025 03:35
@tensorrt-cicd
Copy link
Collaborator

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

auto-merge was automatically disabled August 15, 2025 04:09

Head branch was pushed to by a user without write access

@Yuening-wa Yuening-wa force-pushed the user/yueningl/moe_int8_weight_only_quant_support branch from 45bc810 to e01f35b Compare August 15, 2025 04:09
@Yuening-wa
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #15396 [ run ] triggered by Bot

@yuxianq yuxianq enabled auto-merge (squash) August 15, 2025 10:00
@tensorrt-cicd
Copy link
Collaborator

PR_Github #15396 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #11605 completed with status: 'SUCCESS'

@yuxianq yuxianq merged commit 1f8ae2b 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.

9 participants