[TRTLLM-12288][fix] add the follow-up fix of nemotron nvfp4 on hopper - #14862
[TRTLLM-12288][fix] add the follow-up fix of nemotron nvfp4 on hopper#14862JadoTu wants to merge 6 commits into
Conversation
Signed-off-by: jiant <107457950+JadoTu@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR refines NVFP4 W4A16 support for Hopper GPUs by tightening SM-version hardware detection to [100, 120), disabling FP4 attention output on unsupported SM versions, consolidating dequantization imports to module scope, and adding stricter input validation throughout the dequantization path. ChangesNVFP4 W4A16 Hopper Refinement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tensorrt_llm/_torch/modules/fused_moe/quantization.py`:
- Line 2914: The class currently only sets eplb_support_status =
EplbSupportStatus.NOT_SUPPORTED (metadata) but still inherits
NVFP4FusedMoEMethod.create_weights(), which never calls
_online_eplb_not_supported(), so modules with an attached layer_load_balancer
can still take the shared-weight path; to fix, add a runtime fail-fast check by
overriding create_weights (or adding an __init__ guard) in this class to call
_online_eplb_not_supported() when eplb_support_status ==
EplbSupportStatus.NOT_SUPPORTED and a layer_load_balancer is present (or simply
raise an error), ensuring the code path that would use
NVFP4FusedMoEMethod.create_weights() cannot proceed with EPLB disabled.
In `@tensorrt_llm/_torch/modules/triton_dequant_nvfp4.py`:
- Around line 167-189: The assert-based runtime checks for packed_weight,
scale_linear, active_mask, and weight_scale_2 should be converted to explicit
runtime exceptions so they are not stripped with python -O: replace each assert
with an if condition that raises a ValueError (or a more specific exception)
using the existing error message strings (e.g., check
packed_weight.stride(-1)==1 and raise ValueError("packed_weight innermost stride
must be 1 (contiguous K dim)"), verify scale_linear.dim()==3 and stride(-1)==1
and shape[0]==E, ensure active_mask is 1D contiguous of length E, and ensure
weight_scale_2 is 1D contiguous of length E) to preserve the same validation
behavior at runtime.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 294f06bd-2220-470d-8b51-e7976f10335b
📒 Files selected for processing (5)
tensorrt_llm/_torch/models/modeling_nemotron_h.pytensorrt_llm/_torch/modules/fused_moe/quantization.pytensorrt_llm/_torch/modules/linear.pytensorrt_llm/_torch/modules/triton_dequant_nvfp4.pytests/integration/test_lists/qa/llm_function_core.txt
Signed-off-by: jiant <107457950+JadoTu@users.noreply.github.com>
Signed-off-by: jiant <107457950+JadoTu@users.noreply.github.com>
|
/bot run -disable-fail-fast |
|
PR_Github #51763 Bot args parsing error: usage: /bot [-h] |
|
/bot run --disable-fail-fast |
|
PR_Github #51766 [ run ] triggered by Bot. Commit: |
| # Mirrors what `TRTLLM_ENABLE_ATTENTION_NVFP4_OUTPUT=0` does. | ||
| original_attn_create_weights(self) | ||
| if hasattr(self.attn, 'use_nvfp4_output'): | ||
| self.attn.use_nvfp4_output = lambda *args, **kwargs: False |
There was a problem hiding this comment.
why it is not simple False but a lambda function here?
|
PR_Github #51766 [ run ] completed with state
|
|
/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. |
|
PR_Github #52226 Bot args parsing error: usage: /bot [-h] |
Signed-off-by: jiant <107457950+JadoTu@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #52564 [ run ] triggered by Bot. Commit: |
|
PR_Github #52564 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #52693 [ run ] triggered by Bot. Commit: |
|
PR_Github #52693 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #52929 [ run ] triggered by Bot. Commit: |
|
PR_Github #52929 [ run ] completed with state
|
|
Close this PR as the parent feature is reverted. |
4 participants
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores
Description
This PR is to add the follow-up fix of supporting nemotron nvfp4 on hopper. The link of pr is here.
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.