[None][fix] Fix Qwen image CUDA graph with CFG#16391
Conversation
Signed-off-by: Yibin Li <109242046+yibinl-nvidia@users.noreply.github.com>
|
/bot run |
|
PR_Github #59289 [ run ] triggered by Bot. Commit: |
|
PR_Github #59289 [ run ] completed with state
|
934e2de to
7c24335
Compare
📝 WalkthroughWalkthroughQwenImage CUDA graph denoising now clones CFG predictions before buffer reuse, and integration coverage generates a CUDA-graph-enabled image and compares it with the existing LPIPS golden-image threshold. ChangesQwenImage CUDA graph support
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/bot run |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/integration/defs/examples/visual_gen/test_visual_gen.py (2)
814-815: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate the changed functions.
Both
_generate_qwenimage_lpips_imageandtest_qwenimage_cuda_graph_lpips_against_goldenlack parameter and return annotations.Proposed annotations
-def _generate_qwenimage_lpips_image(model_path, output_path, *, enable_cuda_graph=False): +def _generate_qwenimage_lpips_image( + model_path: str | Path, + output_path: str | Path, + *, + enable_cuda_graph: bool = False, +) -> None: ... -def test_qwenimage_cuda_graph_lpips_against_golden(tmp_path): +def test_qwenimage_cuda_graph_lpips_against_golden(tmp_path: Path) -> None:As per coding guidelines, “Annotate every function.”
Also applies to: 1086-1087
🤖 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 `@tests/integration/defs/examples/visual_gen/test_visual_gen.py` around lines 814 - 815, Annotate both _generate_qwenimage_lpips_image and test_qwenimage_cuda_graph_lpips_against_golden with parameter and return type annotations, covering every declared argument and the function return value. Use the types consistent with surrounding test helpers and preserve the existing behavior and signatures otherwise.Source: Coding guidelines
1086-1105: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftAdd a direct eager-vs-CUDA-graph comparison.
This test only compares the graph output with
qwenimage_lpips_golden.pngusing the existing0.05threshold. It does not directly verify that CUDA-graph output matches eager output, which is the behavior this fix targets; a regression could still pass the indirect golden-image check.Generate an eager reference in this test and run LPIPS against the CUDA-graph result, preferably with a dedicated tighter threshold.
Suggested coverage extension
def test_qwenimage_cuda_graph_lpips_against_golden(tmp_path): + eager_path = tmp_path / "qwenimage_eager_generated.png" + _generate_qwenimage_lpips_image( + _lpips_model_path(QWENIMAGE_MODEL_SUBPATH), + eager_path, + enable_cuda_graph=False, + ) + generated_path = tmp_path / "qwenimage_cuda_graph_generated.png" ... _assert_lpips_below_threshold(score, QWENIMAGE_LPIPS_THRESHOLD) + + eager_score = _run_lpips_eval( + tmp_path, + "qwenimage_cuda_graph_vs_eager", + "image", + QWENIMAGE_LPIPS_PROMPT, + eager_path, + generated_path, + ) + _assert_lpips_below_threshold(eager_score, QWENIMAGE_LPIPS_THRESHOLD)As per path instructions, this integration-test coverage is insufficient for the stated eager-vs-CUDA-graph objective and needs a concrete follow-up in
tests/integration/defs/examples/visual_gen/test_visual_gen.py.🤖 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 `@tests/integration/defs/examples/visual_gen/test_visual_gen.py` around lines 1086 - 1105, Extend test_qwenimage_cuda_graph_lpips_against_golden to generate an eager reference image alongside the CUDA-graph output, then run LPIPS directly between those two generated images. Assert the comparison with a dedicated, tighter eager-vs-CUDA-graph threshold while preserving the existing golden-image coverage as appropriate.Source: Path instructions
🤖 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 `@tests/integration/defs/examples/visual_gen/test_visual_gen.py`:
- Around line 814-815: Annotate both _generate_qwenimage_lpips_image and
test_qwenimage_cuda_graph_lpips_against_golden with parameter and return type
annotations, covering every declared argument and the function return value. Use
the types consistent with surrounding test helpers and preserve the existing
behavior and signatures otherwise.
- Around line 1086-1105: Extend test_qwenimage_cuda_graph_lpips_against_golden
to generate an eager reference image alongside the CUDA-graph output, then run
LPIPS directly between those two generated images. Assert the comparison with a
dedicated, tighter eager-vs-CUDA-graph threshold while preserving the existing
golden-image coverage as appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d75e6b9a-37c0-4445-9fb0-0596920df915
📒 Files selected for processing (2)
tensorrt_llm/_torch/visual_gen/models/qwen_image/pipeline_qwen_image.pytests/integration/defs/examples/visual_gen/test_visual_gen.py
|
PR_Github #59295 [ run ] triggered by Bot. Commit: |
7c24335 to
b67b010
Compare
|
/bot kill |
Signed-off-by: Yibin-Li <109242046+yibinl-nvidia@users.noreply.github.com>
b67b010 to
9f986e5
Compare
|
PR_Github #59297 [ kill ] triggered by Bot. Commit: |
|
PR_Github/16391-7c24335 #59295 was force-killed by a newer pipeline run. |
|
PR_Github #59297 [ kill ] completed with state |
|
/bot run |
|
PR_Github #59299 [ run ] triggered by Bot. Commit: |
|
PR_Github #59299 [ run ] completed with state |
Signed-off-by: Yibin Li <109242046+yibinl-nvidia@users.noreply.github.com>
|
/bot run |
Summary by CodeRabbit
Bug Fixes
Tests
Description
Fix CUDA graph generation for Qwen Image when true CFG is enabled with a negative prompt. The images below compare eager mode and CUDA graph output after the fix.
CUDA graph vs eager LPIPS is 0.000000.
Test Coverage
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.