[None][perf] Add WAN skip-softmax recipe config - #14811
Conversation
93be8cd to
986d18a
Compare
Signed-off-by: Kanghwan Jang <861393+karljang@users.noreply.github.com>
986d18a to
44824ca
Compare
📝 WalkthroughWalkthroughThis PR introduces step-based skip-softmax scheduling for visual generation by adding a ChangesSkip-Softmax Scheduling for Visual Generation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tensorrt_llm/visual_gen/sparse_attention.py (1)
379-386: 💤 Low valueCollapse the duplicated
auto_detectbranches.Both branches now build the formula identically via
_formula_kwargs_from_prefill(prefill); the only thing that differs is the guard. They can be merged into a single check.♻️ Proposed simplification
- if "log_a" in prefill and "b" in prefill: - return SkipSoftmaxConfig.with_calibration( - formula=SkipSoftmaxFormula(**_formula_kwargs_from_prefill(prefill)), - ) - if "a" in prefill and "b" in prefill: - return SkipSoftmaxConfig.with_calibration( - formula=SkipSoftmaxFormula(**_formula_kwargs_from_prefill(prefill)), - ) - - return None + if "b" in prefill and ("log_a" in prefill or "a" in prefill): + return SkipSoftmaxConfig.with_calibration( + formula=SkipSoftmaxFormula(**_formula_kwargs_from_prefill(prefill)), + ) + + return None🤖 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/visual_gen/sparse_attention.py` around lines 379 - 386, Two duplicated branches check prefill guards then call SkipSoftmaxConfig.with_calibration with the same SkipSoftmaxFormula built from _formula_kwargs_from_prefill(prefill); collapse them into one branch. Replace the two separate if blocks with a single condition that tests ("log_a" in prefill or "a" in prefill) and "b" in prefill, and return SkipSoftmaxConfig.with_calibration(formula=SkipSoftmaxFormula(**_formula_kwargs_from_prefill(prefill))). This keeps the same behavior while removing the duplicated construction and guards.
🤖 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.
Nitpick comments:
In `@tensorrt_llm/visual_gen/sparse_attention.py`:
- Around line 379-386: Two duplicated branches check prefill guards then call
SkipSoftmaxConfig.with_calibration with the same SkipSoftmaxFormula built from
_formula_kwargs_from_prefill(prefill); collapse them into one branch. Replace
the two separate if blocks with a single condition that tests ("log_a" in
prefill or "a" in prefill) and "b" in prefill, and return
SkipSoftmaxConfig.with_calibration(formula=SkipSoftmaxFormula(**_formula_kwargs_from_prefill(prefill))).
This keeps the same behavior while removing the duplicated construction and
guards.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 86690e3e-691b-4bcc-9474-7b55216231c0
📒 Files selected for processing (9)
docs/source/features/sparse-attention.mdexamples/visual_gen/configs/wan2.2-t2v-bf16-skip-softmax-8gpu.yamltensorrt_llm/_torch/visual_gen/models/wan/transformer_wan.pytensorrt_llm/_torch/visual_gen/modules/attention.pytensorrt_llm/_torch/visual_gen/pipeline.pytensorrt_llm/visual_gen/sparse_attention.pytests/unittest/_torch/visual_gen/test_attention_integration.pytests/unittest/_torch/visual_gen/test_skip_softmax_config.pytests/unittest/_torch/visual_gen/test_visual_gen_args.py
|
Closing in favor of #14687 |
Summary
first_dense_stepsinto the VisualGen skip-softmax path so early denoising steps can run dense before sparse attention is enabled.sparse.yamlfor calibration/disabled-layer data, withsparse_config_pathremaining as an advanced fallback.Follow-up measurement
Testing
pre-commit run --files <changed files>python3 -m py_compile <changed Python files>git diff --check origin/main...HEADFocused pytest was not run in this non-Docker shell because
mpi4pycould not loadlibmpi.