[None][perf] Enable FLUX2 VisualGen fused NVFP4 SwiGLU path - #16143
Conversation
cfbea35 to
63aae9b
Compare
|
/bot run |
63aae9b to
274c185
Compare
|
B300 follow-up validation for commit The branch now includes merged #15676 from Fixes added after GPU validation
Focused real-op correctness
Focused performanceFLUX.2-dev TP8-local dimensions (
The repaired split fallback measured 0.1505 ms because the layout reorder requires a copy; the producer-fused path is the expected static-NVFP4 route. Local pre-commit, Ruff, formatting, byte-compilation, and diff checks pass after rebasing onto #15676. This was focused real-op/model-construction validation, not a full FLUX.2 checkpoint generation run. |
274c185 to
3c289d1
Compare
Signed-off-by: Alex Steiner <pst2154@users.noreply.github.com>
3c289d1 to
f5c3d58
Compare
📝 WalkthroughWalkthroughFLUX.2 adds hardware- and quantization-gated CUTEDSL/NVFP4 MLP projection paths, propagates the configuration through joint projections and transformer FFNs, and adds unit tests for eligibility guards, tensor ordering, and SM100f behavior. ChangesFLUX.2 FP4 projection acceleration
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Flux2ParallelSelfAttention
participant CUTEDSL_NVFP4_Kernels
participant ToOutProjection
Flux2ParallelSelfAttention->>Flux2ParallelSelfAttention: evaluate FP4/CUTEDSL eligibility
Flux2ParallelSelfAttention->>CUTEDSL_NVFP4_Kernels: quantize and project MLP states
CUTEDSL_NVFP4_Kernels-->>Flux2ParallelSelfAttention: return MLP projection
Flux2ParallelSelfAttention->>ToOutProjection: combine attention and MLP outputs
ToOutProjection-->>Flux2ParallelSelfAttention: return projected output
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tensorrt_llm/_torch/visual_gen/models/flux/attention.py (1)
607-627: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGate weight interleaving with the same CuteDSL eligibility.
use_cute_dsl_blockscaling_mmstill interleavesgate_up_projweights during load, butforward()only uses the CuteDSL path when_can_project_hidden_mlp_with_cute_dsl()passes. Ontp_size == 1, that guard is false, so the split fallback reads an already-interleaved[up | gate]layout as contiguous[gate | up]and returns wrong activations.🤖 Prompt for 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. In `@tensorrt_llm/_torch/visual_gen/models/flux/attention.py` around lines 607 - 627, The weight interleaving controlled by use_cute_dsl_blockscaling_mm must use the same eligibility condition as the forward CuteDSL path. Update the gate_up_proj loading/interleaving logic and its related guard to rely on _can_project_hidden_mlp_with_cute_dsl(), so tp_size == 1 uses the non-interleaved layout expected by the split fallback while eligible CuteDSL execution retains interleaving.
🤖 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/visual_gen/models/flux/transformer_flux2.py`:
- Around line 253-257: Update the use_cute_dsl_swiglu initialization to avoid
unconditionally calling config.get_quant_config() when config may be None. Reuse
the already-resolved self.quant_config as in attention.py::_init_qkv_proj, or
otherwise guard the config access while preserving the existing optional-config
behavior.
---
Outside diff comments:
In `@tensorrt_llm/_torch/visual_gen/models/flux/attention.py`:
- Around line 607-627: The weight interleaving controlled by
use_cute_dsl_blockscaling_mm must use the same eligibility condition as the
forward CuteDSL path. Update the gate_up_proj loading/interleaving logic and its
related guard to rely on _can_project_hidden_mlp_with_cute_dsl(), so tp_size ==
1 uses the non-interleaved layout expected by the split fallback while eligible
CuteDSL execution retains interleaving.
🪄 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: aebde702-b46b-4a28-87e7-365188413298
📒 Files selected for processing (4)
tensorrt_llm/_torch/visual_gen/models/flux/attention.pytensorrt_llm/_torch/visual_gen/models/flux/joint_proj.pytensorrt_llm/_torch/visual_gen/models/flux/transformer_flux2.pytests/unittest/_torch/visual_gen/test_flux_transformer.py
|
@pst2154, could you please resolve any conflicts? Also, I’m curious if you had a chance to verify the e2e LPIPS results as well? |
Signed-off-by: Alex Steiner <asteiner@nvidia.com>
|
/bot run --disable-fail-fast |
|
Resolved the conflict against current main in |
|
Fused SwiGLU paths have no numerical coverage — only guard tests The new unit tests exercise only the dispatch guards; every one of them mocks out the actual fused op, so nothing validates that the fused math matches the eager reference. E.g. That equivalence is load-bearing and subtle: the Suggestion: add an SM100/103-gated parity test that runs the real ops and asserts each fast path matches the eager This keeps the opposite-convention invariant from regressing unnoticed. |
Signed-off-by: Alex Steiner <asteiner@nvidia.com>
|
@luyiyun1021 @zhenhuaw-me The requested numerical coverage is now in |
|
/bot run |
|
/bot run --disable-fail-fast |
|
FYI @pst2154 — the three earlier I have triggered it from an allowlisted account; the pipeline is running now. Worth opening a small PR to add yourself to the allowlist in |
|
PR_Github #61968 [ run ] triggered by Bot. Commit: |
|
PR_Github #61968 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
1 similar comment
|
/bot run --disable-fail-fast |
|
PR_Github #62012 [ run ] triggered by Bot. Commit: |
|
PR_Github #62012 [ run ] completed with state |
Summary
[gate | up]layout to the quantizer's[up | gate]contract.main, which independently restores fused FLUX QK-norm + RoPE at TP=1. That path is complementary to this PR's TP>1 FLUX.2 NVFP4 producer optimization.Checks
upstream/main, including merged [None][fix] visual_gen FLUX: enable fused DiT QK-norm + RoPE by default #15676.f5c3d58d6d.B300 validation
Using
nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc18on SM103:M=4096, K=6144, N=2304): 0.1028 ms baseline versus 0.0658 ms producer-fused (1.56x faster).Scope note
Validation used focused real operators and model construction, not a full FLUX.2 checkpoint generation run. #15676 separately includes TP1 end-to-end FLUX.2-dev BF16 validation.
Summary by CodeRabbit
New Features
Tests