[TRTLLM-14557][test] Add single-device visual-gen feature accuracy regression tests#16683
[TRTLLM-14557][test] Add single-device visual-gen feature accuracy regression tests#16683zhaoyuanh-nvidia wants to merge 1 commit into
Conversation
455c544 to
92af6ba
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughVisualGen integration coverage is reorganized into model-specific Cosmos3, FLUX, LTX-2, Qwen-Image, and WAN modules with shared LPIPS/VBench utilities, golden metadata, fixtures, and updated test-list ownership. ChangesVisualGen integration coverage
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TestModule
participant PipelineLoader
participant VisualGen
participant GoldenMedia
participant LPIPS
TestModule->>PipelineLoader: load model with feature configuration
PipelineLoader-->>TestModule: return configured pipeline
TestModule->>VisualGen: generate image or video
VisualGen-->>TestModule: return generated media
TestModule->>GoldenMedia: load reference artifact
TestModule->>LPIPS: compare generated media with reference
LPIPS-->>TestModule: return regression score
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 4
🧹 Nitpick comments (3)
tests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.py (1)
201-257: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared single-device feature-pipeline lifecycle into
visual_gen_test_utils. All three per-model "generate feature image" functions duplicate the same skip-warmup derivation, deterministic-algorithms context, resolved-config/feature assertions, forward call, and pipeline/cuda cleanup try/finally — only the model-specific prompt kwargs and frame-extraction line differ.
tests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.py#L201-L257: replace the pipeline build/assert/forward/cleanup boilerplate in_generate_cosmos3_feature_imagewith a shared context-manager/helper fromvisual_gen_test_utils.pythat takes a model-specificforward(pipeline)callback.tests/integration/defs/examples/visual_gen/test_visual_gen_flux.py#L207-L252: apply the same shared helper in_generate_flux_image.tests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.py#L155-L201: apply the same shared helper in_generate_qwenimage_feature_image.🤖 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_cosmos3.py` around lines 201 - 257, Extract the duplicated single-device feature-pipeline lifecycle into a shared helper or context manager in visual_gen_test_utils.py, accepting the model-specific forward(pipeline) callback while preserving skip-warmup derivation, deterministic-algorithm handling, configuration and feature assertions, and pipeline/CUDA cleanup. Update _generate_cosmos3_feature_image in tests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.py (lines 201-257), _generate_flux_image in tests/integration/defs/examples/visual_gen/test_visual_gen_flux.py (lines 207-252), and _generate_qwenimage_feature_image in tests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.py (lines 155-201) to use the shared lifecycle, retaining each function’s model-specific forward arguments and frame extraction.tests/integration/defs/examples/visual_gen/test_visual_gen_wan.py (1)
71-75: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCheckpoint subdirectory strings duplicated instead of reusing module constants.
"Wan2.1-T2V-1.3B-Diffusers"is hardcoded inWAN_MODEL_SPECS(Line 171) and again in thewan21_bf16_video_pathfixture (Line 332), despiteWAN_T2V_MODEL_SUBPATH(Line 71) already holding this value. Similarly,"Wan2.2-T2V-A14B-Diffusers"appears both inWAN_MODEL_SPECS(Line 184) andwan22_bf16_video_path(Line 355) with no shared constant. Consolidating these reduces the risk of the values drifting apart on future edits.Also applies to: 168-195, 332-332, 355-355
🤖 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_wan.py` around lines 71 - 75, Replace the duplicated checkpoint subdirectory literals in WAN_MODEL_SPECS and the wan21_bf16_video_path fixture with the existing WAN_T2V_MODEL_SUBPATH constant. Add and reuse a shared constant for the Wan2.2 T2V A14B Diffusers subdirectory in WAN_MODEL_SPECS and wan22_bf16_video_path, preserving the current values.tests/integration/defs/examples/visual_gen/test_visual_gen_ltx2.py (1)
151-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale "Initially None" comment.
The comment says scores are "Initially None — first CI run is a baseline that prints scores for capture," but both dicts already contain concrete calibrated values. This is misleading for future readers trying to determine whether these are placeholders still awaiting capture.
✏️ Proposed fix
-# Golden VBench scores for two-stage pipeline variants. -# Initially None — first CI run is a baseline that prints scores for capture. +# Golden VBench scores for two-stage pipeline variants, captured from an +# initial baseline CI run.🤖 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_ltx2.py` around lines 151 - 160, Update the comment above VBENCH_LTX2_TWO_STAGE_BF16_GOLDEN_SCORES to describe these entries as calibrated golden baseline scores, and remove the stale claim that they are initially None or awaiting first-run capture. Apply the same clarification to any nearby equivalent score dictionary comments.
🤖 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 `@tests/integration/defs/examples/visual_gen/test_visual_gen_ltx2.py`:
- Around line 472-544: Update _generate_ltx2_two_stage_video to resolve
text_encoder_path through the existing _ltx2_lpips_text_encoder_path() helper,
preserving its primary and gemma/ fallback lookup behavior before the
asset-existence checks and VisualGen configuration.
In `@tests/integration/defs/examples/visual_gen/test_visual_gen_wan.py`:
- Around line 297-298: Confirm that VBENCH_WAN22_BF16_GOLDEN_SCORES contains
final calibrated values from bf16 baseline runs before merging; if not, replace
the interim values with the referenced baseline scores and remove the TODO, or
explicitly track the remaining calibration work instead of leaving it
unresolved.
- Around line 692-733: The model-availability check in test_wan_t2v_example
should skip rather than fail when the NVFP4 checkpoint is missing. Replace the
hard assertion on model_path with the same pytest.skip pattern used by
test_wan_i2v_example and the other model-lookup helpers, preserving the existing
path and guidance message.
- Around line 252-273: Update the Wan 2.1 VSA case in the test-case construction
to use the WAN21 prompt and negative-prompt constants, and replace the hardcoded
guidance_scale value with the shared guidance-scale constant used by the other
Wan cases. Keep the existing wan21-vsa identifiers, dimensions, thresholds, and
feature configuration unchanged.
---
Nitpick comments:
In `@tests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.py`:
- Around line 201-257: Extract the duplicated single-device feature-pipeline
lifecycle into a shared helper or context manager in visual_gen_test_utils.py,
accepting the model-specific forward(pipeline) callback while preserving
skip-warmup derivation, deterministic-algorithm handling, configuration and
feature assertions, and pipeline/CUDA cleanup. Update
_generate_cosmos3_feature_image in
tests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.py (lines
201-257), _generate_flux_image in
tests/integration/defs/examples/visual_gen/test_visual_gen_flux.py (lines
207-252), and _generate_qwenimage_feature_image in
tests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.py (lines
155-201) to use the shared lifecycle, retaining each function’s model-specific
forward arguments and frame extraction.
In `@tests/integration/defs/examples/visual_gen/test_visual_gen_ltx2.py`:
- Around line 151-160: Update the comment above
VBENCH_LTX2_TWO_STAGE_BF16_GOLDEN_SCORES to describe these entries as calibrated
golden baseline scores, and remove the stale claim that they are initially None
or awaiting first-run capture. Apply the same clarification to any nearby
equivalent score dictionary comments.
In `@tests/integration/defs/examples/visual_gen/test_visual_gen_wan.py`:
- Around line 71-75: Replace the duplicated checkpoint subdirectory literals in
WAN_MODEL_SPECS and the wan21_bf16_video_path fixture with the existing
WAN_T2V_MODEL_SUBPATH constant. Add and reuse a shared constant for the Wan2.2
T2V A14B Diffusers subdirectory in WAN_MODEL_SPECS and wan22_bf16_video_path,
preserving the current values.
🪄 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: b7af6981-1dd8-41ba-bcc0-1cebd5453d3a
⛔ Files ignored due to path filters (1)
tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/visual_gen_lpips_golden_media.zipis excluded by!**/*.zip
📒 Files selected for processing (25)
.github/CODEOWNERStests/integration/defs/.test_durationstests/integration/defs/examples/visual_gen/conftest.pytests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/cosmos3_nano_feature_eager_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux1_50step_eager_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_50step_eager_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_feature_eager_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_feature_eager_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_12step_eager_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_vsa_dense_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan22_12step_eager_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/test_visual_gen.pytests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.pytests/integration/defs/examples/visual_gen/test_visual_gen_flux.pytests/integration/defs/examples/visual_gen/test_visual_gen_ltx2.pytests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.pytests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.pytests/integration/defs/examples/visual_gen/test_visual_gen_wan.pytests/integration/defs/examples/visual_gen/visual_gen_test_utils.pytests/integration/test_lists/test-db/l0_b200.ymltests/integration/test_lists/test-db/l0_dgx_b200.ymltests/integration/test_lists/test-db/l0_gh200.ymltests/integration/test_lists/test-db/l0_h100.ymltests/integration/test_lists/test-db/l0_l40s.ymltests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
- tests/integration/defs/examples/visual_gen/test_visual_gen.py
BowenFu
left a comment
BowenFu
left a comment
There was a problem hiding this comment.
LGTM — test-only refactor. Splits the monolithic test_visual_gen.py into per-model modules (+shared utils); no product code touched (all 26 files under tests/ + CODEOWNERS). New .py files carry SPDX/Apache 2026 headers; waiver NVBug refs preserved through the path renames. The new B200 accuracy-vs-golden tests are the visual-gen owner's domain and gated by the PR's own pre-merge run before it can land — not a product regression.
92af6ba to
f3fa199
Compare
|
/bot run |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/integration/defs/examples/visual_gen/visual_gen_test_utils.py (1)
613-629: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer narrower exception scope in the LPIPS eval wrappers.
Both wrappers catch bare
Exceptiononly to re-raise viapytest.fail. Consider narrowing to the concrete failure types (e.g.RuntimeError,ValueError,OSError) so genuinely unexpected errors aren't masked into a generic failure message.As per coding guidelines: "Catch the narrowest possible exceptions, keep duck-typing try blocks minimal."
🤖 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/visual_gen_test_utils.py` around lines 613 - 629, In the LPIPS evaluation wrappers around _lpips_deterministic_algorithms(), scorer.score(), and scorer.score_video(), replace the broad Exception handlers with the narrowest concrete exceptions these operations can raise, such as RuntimeError, ValueError, and OSError. Keep the try blocks limited to the scoring operation and preserve the existing pytest.fail messages for those expected failures, allowing unexpected exceptions to propagate.Source: Coding guidelines
🤖 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/visual_gen_test_utils.py`:
- Around line 613-629: In the LPIPS evaluation wrappers around
_lpips_deterministic_algorithms(), scorer.score(), and scorer.score_video(),
replace the broad Exception handlers with the narrowest concrete exceptions
these operations can raise, such as RuntimeError, ValueError, and OSError. Keep
the try blocks limited to the scoring operation and preserve the existing
pytest.fail messages for those expected failures, allowing unexpected exceptions
to propagate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9a9c146e-7aaa-49ad-95a7-da8ef37e4ef0
⛔ Files ignored due to path filters (1)
tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/visual_gen_lpips_golden_media.zipis excluded by!**/*.zip
📒 Files selected for processing (25)
.github/CODEOWNERStests/integration/defs/.test_durationstests/integration/defs/examples/visual_gen/conftest.pytests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/cosmos3_nano_feature_eager_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux1_50step_eager_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_50step_eager_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_feature_eager_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_feature_eager_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_12step_eager_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_vsa_dense_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan22_12step_eager_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/test_visual_gen.pytests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.pytests/integration/defs/examples/visual_gen/test_visual_gen_flux.pytests/integration/defs/examples/visual_gen/test_visual_gen_ltx2.pytests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.pytests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.pytests/integration/defs/examples/visual_gen/test_visual_gen_wan.pytests/integration/defs/examples/visual_gen/visual_gen_test_utils.pytests/integration/test_lists/test-db/l0_b200.ymltests/integration/test_lists/test-db/l0_dgx_b200.ymltests/integration/test_lists/test-db/l0_gh200.ymltests/integration/test_lists/test-db/l0_h100.ymltests/integration/test_lists/test-db/l0_l40s.ymltests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
- tests/integration/defs/examples/visual_gen/test_visual_gen.py
🚧 Files skipped from review as they are similar to previous changes (20)
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_feature_eager_lpips_golden_video.json
- tests/integration/test_lists/test-db/l0_h100.yml
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/cosmos3_nano_feature_eager_lpips_golden.json
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_12step_eager_lpips_golden_video.json
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux1_50step_eager_lpips_golden.json
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_feature_eager_lpips_golden.json
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_vsa_dense_lpips_golden_video.json
- .github/CODEOWNERS
- tests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.py
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_50step_eager_lpips_golden.json
- tests/integration/test_lists/waives.txt
- tests/integration/test_lists/test-db/l0_dgx_b200.yml
- tests/integration/test_lists/test-db/l0_b200.yml
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan22_12step_eager_lpips_golden_video.json
- tests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.py
- tests/integration/defs/examples/visual_gen/test_visual_gen_flux.py
- tests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.py
- tests/integration/defs/examples/visual_gen/conftest.py
- tests/integration/defs/examples/visual_gen/test_visual_gen_wan.py
- tests/integration/defs/examples/visual_gen/test_visual_gen_ltx2.py
|
/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. |
|
/bot run --disable-fail-fast |
|
/bot run |
|
PR_Github #61053 [ run ] triggered by Bot. Commit: |
|
PR_Github #61053 [ run ] completed with state |
f3fa199 to
57140d0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/integration/defs/examples/visual_gen/test_visual_gen_wan.py (1)
136-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
WanModelSpecandWanAccuracyCaseduplicate nearly all fields.Both dataclasses carry
checkpoint_subdir/golden_file,prompt,negative_prompt,height,width,num_frames,num_inference_steps,guidance_scale. ComposingWanAccuracyCasefrom aWanModelSpec(or reusing it directly plus the feature/threshold-specific fields) would remove this duplication and reduce the risk of the two structs drifting.🤖 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_wan.py` around lines 136 - 166, Refactor WanAccuracyCase and WanModelSpec to share their common fields instead of duplicating them, preferably by composing WanAccuracyCase with a WanModelSpec and retaining only features, lpips_threshold, and vsa_sparsity-specific data in the accuracy case. Update construction and access sites accordingly while preserving existing behavior and field values.
🤖 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 `@tests/integration/defs/examples/visual_gen/visual_gen_test_utils.py`:
- Around line 440-446: Update score_video to validate that generated and
reference contain the same number of frames before invoking _get_model; reject
mismatched lengths explicitly instead of truncating with paired_frame_count.
Preserve the existing scoring path for equal-length videos.
---
Nitpick comments:
In `@tests/integration/defs/examples/visual_gen/test_visual_gen_wan.py`:
- Around line 136-166: Refactor WanAccuracyCase and WanModelSpec to share their
common fields instead of duplicating them, preferably by composing
WanAccuracyCase with a WanModelSpec and retaining only features,
lpips_threshold, and vsa_sparsity-specific data in the accuracy case. Update
construction and access sites accordingly while preserving existing behavior and
field values.
🪄 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: 1d5dd43b-e292-442d-aaad-5a9c30e93907
⛔ Files ignored due to path filters (1)
tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/visual_gen_lpips_golden_media.zipis excluded by!**/*.zip
📒 Files selected for processing (25)
.github/CODEOWNERStests/integration/defs/.test_durationstests/integration/defs/examples/visual_gen/conftest.pytests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/cosmos3_nano_feature_eager_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux1_50step_eager_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_50step_eager_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_feature_eager_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_feature_eager_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_12step_eager_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_vsa_dense_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan22_12step_eager_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/test_visual_gen.pytests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.pytests/integration/defs/examples/visual_gen/test_visual_gen_flux.pytests/integration/defs/examples/visual_gen/test_visual_gen_ltx2.pytests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.pytests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.pytests/integration/defs/examples/visual_gen/test_visual_gen_wan.pytests/integration/defs/examples/visual_gen/visual_gen_test_utils.pytests/integration/test_lists/test-db/l0_b200.ymltests/integration/test_lists/test-db/l0_dgx_b200.ymltests/integration/test_lists/test-db/l0_gh200.ymltests/integration/test_lists/test-db/l0_h100.ymltests/integration/test_lists/test-db/l0_l40s.ymltests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
- tests/integration/defs/examples/visual_gen/test_visual_gen.py
🚧 Files skipped from review as they are similar to previous changes (21)
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan22_12step_eager_lpips_golden_video.json
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/cosmos3_nano_feature_eager_lpips_golden.json
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_50step_eager_lpips_golden.json
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux1_50step_eager_lpips_golden.json
- tests/integration/test_lists/test-db/l0_h100.yml
- tests/integration/test_lists/test-db/l0_l40s.yml
- .github/CODEOWNERS
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_12step_eager_lpips_golden_video.json
- tests/integration/test_lists/test-db/l0_dgx_b200.yml
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_vsa_dense_lpips_golden_video.json
- tests/integration/test_lists/waives.txt
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_feature_eager_lpips_golden_video.json
- tests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.py
- tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_feature_eager_lpips_golden.json
- tests/integration/defs/examples/visual_gen/test_visual_gen_flux.py
- tests/integration/defs/.test_durations
- tests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.py
- tests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.py
- tests/integration/test_lists/test-db/l0_b200.yml
- tests/integration/defs/examples/visual_gen/conftest.py
- tests/integration/defs/examples/visual_gen/test_visual_gen_ltx2.py
|
/bot run |
|
PR_Github #61075 [ run ] triggered by Bot. Commit: |
|
PR_Github #61075 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #61079 [ run ] triggered by Bot. Commit: |
|
PR_Github #61079 [ run ] completed with state
|
yibinl-nvidia
changed the title
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def vbench_repo_root(llm_venv): |
There was a problem hiding this comment.
We can remove VBench tests
| - examples/visual_gen/test_visual_gen.py::test_cosmos3_example | ||
| - examples/visual_gen/test_visual_gen.py::test_qwen_image_example | ||
| # - examples/visual_gen/test_visual_gen.py | ||
| - examples/visual_gen/test_visual_gen_wan.py::test_wan_t2v_example |
There was a problem hiding this comment.
do we need to add these tests into qa test list?
57140d0 to
34fcdad
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
tests/integration/defs/examples/visual_gen/visual_gen_test_utils.py (3)
512-522: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound the child join so a hung teardown fails the test instead of the job.
The status has already been received at this point, so an unbounded
join()only ever waits on child teardown (CUDA context destruction, inductor workers, atexit). If that wedges, the pytest worker hangs until the whole CI job times out with no attribution.♻️ Suggested bounded join with escalation
- process.join() - exitcode = process.exitcode - process.close() + process.join(timeout=300) + if process.is_alive(): + process.terminate() + process.join(timeout=30) + if process.is_alive(): + process.kill() + process.join() + exitcode = process.exitcode + process.close()🤖 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/visual_gen_test_utils.py` around lines 512 - 522, Update the child-process cleanup around process.join() in the visual generation test utility to use a bounded join timeout. If the process remains alive, escalate by terminating it, wait again with a bounded timeout, and force-kill if necessary before closing it, then fail the test with the existing status and exit-code handling.
717-727: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the DINO clone like VBench is pinned.
VBench is fetched at a fixed commit, but DINO tracks
main. Since DINO backs the subject/background-consistency dimensions, upstream drift can move scores against fixed goldens and reintroduce the non-reproducibility the VBench pin was added to avoid.🤖 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/visual_gen_test_utils.py` around lines 717 - 727, Update _precache_dino_for_torch_hub so the git clone checks out a fixed DINO commit or tag instead of tracking the moving main branch. Define or reuse a pinned revision consistent with the repository’s existing dependency-pinning pattern, and pass that revision to the clone operation while preserving the current cache directory behavior.
618-625: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGate the base64 dump behind an opt-in flag.
The artifact is already copied into
--output-dir; additionally streaming up to ~11 MB of base64 (thousands of printed lines) into the test log on every LPIPS failure makes CI logs hard to use. Suggest requiring an explicit env opt-in.♻️ Suggested opt-in gate
- size = os.path.getsize(dest) - if size <= 8 * 1024 * 1024: + if os.environ.get("TRTLLM_LPIPS_DUMP_BASE64") != "1": + return + size = os.path.getsize(dest) + if size <= 8 * 1024 * 1024:🤖 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/visual_gen_test_utils.py` around lines 618 - 625, Gate the base64 encoding and printing block near the artifact size check behind an explicit environment-variable opt-in, such as a clearly named LPIPS dump flag. Preserve the existing 8 MiB size limit and output format when the flag is enabled, while skipping all base64 log output by default; continue copying the artifact to the output directory.tests/integration/defs/examples/visual_gen/test_visual_gen_wan.py (1)
529-571: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate model-symlink setup between
test_visual_gen_quickstartandtest_visual_gen_api_walkthrough.Both tests repeat the same scratch-space lookup, skip-if-missing check, and symlink creation for
WAN_T2V_MODEL_SUBPATH. Extracting a small helper (e.g.,_ensure_wan_t2v_model_symlink(llm_venv)) would remove this duplication.♻️ Proposed helper extraction
+def _ensure_wan_t2v_symlink(llm_venv): + scratch_space = conftest.llm_models_root() + model_src = os.path.join(scratch_space, WAN_T2V_MODEL_SUBPATH) + if not os.path.isdir(model_src): + pytest.skip( + f"Model not found: {model_src} " + f"(set LLM_MODELS_ROOT or place {WAN_T2V_MODEL_SUBPATH} under scratch)" + ) + model_dst = os.path.join(llm_venv.get_working_directory(), "Wan-AI", WAN_T2V_MODEL_SUBPATH) + if not os.path.islink(model_dst): + os.makedirs(os.path.dirname(model_dst), exist_ok=True) + os.symlink(model_src, model_dst, target_is_directory=True) + + def test_visual_gen_quickstart(_visual_gen_deps, llm_root, llm_venv): """Run examples/visual_gen/quickstart_example.py end-to-end.""" - scratch_space = conftest.llm_models_root() - model_src = os.path.join(scratch_space, WAN_T2V_MODEL_SUBPATH) - if not os.path.isdir(model_src): - pytest.skip( - f"Model not found: {model_src} " - f"(set LLM_MODELS_ROOT or place {WAN_T2V_MODEL_SUBPATH} under scratch)" - ) - - model_dst = os.path.join(llm_venv.get_working_directory(), "Wan-AI", WAN_T2V_MODEL_SUBPATH) - if not os.path.islink(model_dst): - os.makedirs(os.path.dirname(model_dst), exist_ok=True) - os.symlink(model_src, model_dst, target_is_directory=True) + _ensure_wan_t2v_symlink(llm_venv)🤖 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_wan.py` around lines 529 - 571, Extract the repeated WAN model setup from test_visual_gen_quickstart and test_visual_gen_api_walkthrough into a shared helper such as _ensure_wan_t2v_model_symlink(llm_venv). Keep the scratch-space lookup, missing-model skip message, destination creation, and symlink behavior unchanged, then call the helper from both tests before running their respective scripts.
🤖 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
`@tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_fp8_blockwise_lpips_golden.json`:
- Line 2: Align all listed golden fixtures with the shared LPIPS schema by
renaming image to reference_image_path in
tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_fp8_blockwise_lpips_golden.json:2-2,
flux2_nvfp4_lpips_golden.json:2-2, qwenimage_cuda_graph_lpips_golden.json:2-2,
qwenimage_fp8_blockwise_lpips_golden.json:2-2, and
qwenimage_nvfp4_lpips_golden.json:2-2; rename video to reference_video_path in
ltx2_cuda_graph_lpips_golden_video.json:2-2,
ltx2_fp8_blockwise_lpips_golden_video.json:2-2, and
ltx2_nvfp4_lpips_golden_video.json:2-2, preserving each existing path value.
In `@tests/integration/defs/examples/visual_gen/test_visual_gen_flux.py`:
- Around line 119-121: Reformat the golden_file assignments in
tests/integration/defs/examples/visual_gen/test_visual_gen_flux.py:119-121,
tests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.py:100-102,
and
tests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.py:92-94
by removing the redundant parentheses and placing each f-string on one line,
preserving the existing filename expressions.
In `@tests/integration/defs/examples/visual_gen/test_visual_gen_wan.py`:
- Around line 80-107: Add a seed field to WanModelSpec and WanAccuracyCase,
assign WAN21_LPIPS_SEED or WAN22_LPIPS_SEED in each WAN_MODEL_SPECS entry, and
propagate it when constructing WAN_ACCURACY_CASES. Update
_generate_wan_feature_video to use the case’s seed instead of hardcoding
WAN21_LPIPS_SEED, preserving the correct seed for both WAN 2.1 and WAN 2.2
cases.
---
Nitpick comments:
In `@tests/integration/defs/examples/visual_gen/test_visual_gen_wan.py`:
- Around line 529-571: Extract the repeated WAN model setup from
test_visual_gen_quickstart and test_visual_gen_api_walkthrough into a shared
helper such as _ensure_wan_t2v_model_symlink(llm_venv). Keep the scratch-space
lookup, missing-model skip message, destination creation, and symlink behavior
unchanged, then call the helper from both tests before running their respective
scripts.
In `@tests/integration/defs/examples/visual_gen/visual_gen_test_utils.py`:
- Around line 512-522: Update the child-process cleanup around process.join() in
the visual generation test utility to use a bounded join timeout. If the process
remains alive, escalate by terminating it, wait again with a bounded timeout,
and force-kill if necessary before closing it, then fail the test with the
existing status and exit-code handling.
- Around line 717-727: Update _precache_dino_for_torch_hub so the git clone
checks out a fixed DINO commit or tag instead of tracking the moving main
branch. Define or reuse a pinned revision consistent with the repository’s
existing dependency-pinning pattern, and pass that revision to the clone
operation while preserving the current cache directory behavior.
- Around line 618-625: Gate the base64 encoding and printing block near the
artifact size check behind an explicit environment-variable opt-in, such as a
clearly named LPIPS dump flag. Preserve the existing 8 MiB size limit and output
format when the flag is enabled, while skipping all base64 log output by
default; continue copying the artifact to the output directory.
🪄 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: cbc21b1d-4be1-4035-82b3-9d1a234a001b
⛔ Files ignored due to path filters (1)
tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/visual_gen_lpips_golden_media.zipis excluded by!**/*.zip
📒 Files selected for processing (37)
.github/CODEOWNERStests/integration/defs/.test_durationstests/integration/defs/examples/visual_gen/conftest.pytests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/cosmos3_nano_fp8_blockwise_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/cosmos3_nano_nvfp4_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux1_cuda_graph_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux1_fp8_blockwise_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux1_nvfp4_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_cuda_graph_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_fp8_blockwise_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_nvfp4_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_cuda_graph_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_fp8_blockwise_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_nvfp4_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_cuda_graph_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_fp8_blockwise_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_nvfp4_lpips_golden.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_cuda_graph_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_fp8_blockwise_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan21_nvfp4_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan22_cuda_graph_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan22_fp8_blockwise_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan22_nvfp4_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/test_visual_gen.pytests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.pytests/integration/defs/examples/visual_gen/test_visual_gen_flux.pytests/integration/defs/examples/visual_gen/test_visual_gen_ltx2.pytests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.pytests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.pytests/integration/defs/examples/visual_gen/test_visual_gen_wan.pytests/integration/defs/examples/visual_gen/visual_gen_test_utils.pytests/integration/test_lists/test-db/l0_b200.ymltests/integration/test_lists/test-db/l0_dgx_b200.ymltests/integration/test_lists/test-db/l0_gh200.ymltests/integration/test_lists/test-db/l0_h100.ymltests/integration/test_lists/test-db/l0_l40s.ymltests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
- tests/integration/defs/examples/visual_gen/test_visual_gen.py
🚧 Files skipped from review as they are similar to previous changes (9)
- .github/CODEOWNERS
- tests/integration/test_lists/test-db/l0_gh200.yml
- tests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.py
- tests/integration/test_lists/test-db/l0_dgx_b200.yml
- tests/integration/test_lists/test-db/l0_h100.yml
- tests/integration/test_lists/waives.txt
- tests/integration/defs/examples/visual_gen/conftest.py
- tests/integration/defs/.test_durations
- tests/integration/defs/examples/visual_gen/test_visual_gen_ltx2.py
| @@ -0,0 +1,21 @@ | |||
| { | |||
| "image": "flux2_fp8_blockwise_lpips_golden.png", | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align all golden fixtures with the shared LPIPS dataset schema.
The loader expects reference_image_path or reference_video_path, but these fixtures use image or video, causing direct loading to fail before scoring.
tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_fp8_blockwise_lpips_golden.json#L2-L2: renameimagetoreference_image_path.tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_nvfp4_lpips_golden.json#L2-L2: renameimagetoreference_image_path.tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_cuda_graph_lpips_golden_video.json#L2-L2: renamevideotoreference_video_path.tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_fp8_blockwise_lpips_golden_video.json#L2-L2: renamevideotoreference_video_path.tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_nvfp4_lpips_golden_video.json#L2-L2: renamevideotoreference_video_path.tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_cuda_graph_lpips_golden.json#L2-L2: renameimagetoreference_image_path.tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_fp8_blockwise_lpips_golden.json#L2-L2: renameimagetoreference_image_path.tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_nvfp4_lpips_golden.json#L2-L2: renameimagetoreference_image_path.
📍 Affects 8 files
tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_fp8_blockwise_lpips_golden.json#L2-L2(this comment)tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_nvfp4_lpips_golden.json#L2-L2tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_cuda_graph_lpips_golden_video.json#L2-L2tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_fp8_blockwise_lpips_golden_video.json#L2-L2tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/ltx2_nvfp4_lpips_golden_video.json#L2-L2tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_cuda_graph_lpips_golden.json#L2-L2tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_fp8_blockwise_lpips_golden.json#L2-L2tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/qwenimage_nvfp4_lpips_golden.json#L2-L2
🤖 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/golden/visual_gen_lpips/flux2_fp8_blockwise_lpips_golden.json`
at line 2, Align all listed golden fixtures with the shared LPIPS schema by
renaming image to reference_image_path in
tests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/flux2_fp8_blockwise_lpips_golden.json:2-2,
flux2_nvfp4_lpips_golden.json:2-2, qwenimage_cuda_graph_lpips_golden.json:2-2,
qwenimage_fp8_blockwise_lpips_golden.json:2-2, and
qwenimage_nvfp4_lpips_golden.json:2-2; rename video to reference_video_path in
ltx2_cuda_graph_lpips_golden_video.json:2-2,
ltx2_fp8_blockwise_lpips_golden_video.json:2-2, and
ltx2_nvfp4_lpips_golden_video.json:2-2, preserving each existing path value.
| golden_file=( | ||
| f"{model.id}_{profile.id.replace('-', '_')}_lpips_golden.png" | ||
| ), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
ruff-format gate fails on the manually wrapped golden_file f-strings. All three accuracy-case builders wrap the golden_file= f-string in redundant parentheses that ruff-format collapses onto one line, which is why the pre-commit hook reports modified files and the Release Checks job fails.
tests/integration/defs/examples/visual_gen/test_visual_gen_flux.py#L119-L121: collapsegolden_file=(f"{model.id}_{profile.id.replace('-', '_')}_lpips_golden.png")to a single line.tests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.py#L100-L102: collapse thecosmos3_nano_...golden_filef-string to a single line.tests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.py#L92-L94: collapse theqwenimage_...golden_filef-string to a single line.
Running pre-commit run ruff-format --files <these files> fixes all three (and the other reported files) mechanically.
📍 Affects 3 files
tests/integration/defs/examples/visual_gen/test_visual_gen_flux.py#L119-L121(this comment)tests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.py#L100-L102tests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.py#L92-L94
🤖 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_flux.py` around
lines 119 - 121, Reformat the golden_file assignments in
tests/integration/defs/examples/visual_gen/test_visual_gen_flux.py:119-121,
tests/integration/defs/examples/visual_gen/test_visual_gen_cosmos3.py:100-102,
and
tests/integration/defs/examples/visual_gen/test_visual_gen_qwen_image.py:92-94
by removing the redundant parentheses and placing each f-string on one line,
preserving the existing filename expressions.
Source: Pipeline failures
| @dataclass(frozen=True) | ||
| class WanModelSpec: | ||
| id: str | ||
| checkpoint_subdir: str | ||
| prompt: str | ||
| negative_prompt: str | None | ||
| height: int | ||
| width: int | ||
| num_frames: int | ||
| num_inference_steps: int | ||
| guidance_scale: float | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class WanAccuracyCase: | ||
| id: str | ||
| model_id: str | ||
| checkpoint_subdir: str | ||
| golden_file: str | ||
| prompt: str | ||
| negative_prompt: str | None | ||
| height: int | ||
| width: int | ||
| num_frames: int | ||
| num_inference_steps: int | ||
| guidance_scale: float | ||
| features: FeatureConfigState | ||
| lpips_threshold: float |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Feature-accuracy videos always use the WAN 2.1 seed, even for WAN 2.2 cases.
WanModelSpec/WanAccuracyCase carry no seed field, and _generate_wan_feature_video hardcodes seed=WAN21_LPIPS_SEED (Line 353) for every case in WAN_ACCURACY_CASES, which includes both wan21-* and wan22-* combinations built from WAN_MODEL_SPECS. If the WAN 2.2 goldens were captured with WAN22_LPIPS_SEED (as the wan22_bf16_video_path fixture uses at Line 266), the WAN 2.2 feature-accuracy tests generate candidates with the wrong seed relative to their golden, risking spurious LPIPS mismatches or, if it happens to pass, a check that isn't validating what it claims.
🐛 Proposed fix: add per-model seed and thread it through
`@dataclass`(frozen=True)
class WanModelSpec:
id: str
checkpoint_subdir: str
prompt: str
negative_prompt: str | None
height: int
width: int
num_frames: int
num_inference_steps: int
guidance_scale: float
+ seed: int
`@dataclass`(frozen=True)
class WanAccuracyCase:
id: str
model_id: str
checkpoint_subdir: str
golden_file: str
prompt: str
negative_prompt: str | None
height: int
width: int
num_frames: int
num_inference_steps: int
guidance_scale: float
+ seed: int
features: FeatureConfigState
lpips_threshold: float WanModelSpec(
id="wan21",
...
guidance_scale=WAN21_LPIPS_GUIDANCE_SCALE,
+ seed=WAN21_LPIPS_SEED,
),
WanModelSpec(
id="wan22",
...
guidance_scale=WAN22_LPIPS_GUIDANCE_SCALE,
+ seed=WAN22_LPIPS_SEED,
), guidance_scale=model.guidance_scale,
+ seed=model.seed,
features=features, result = pipeline.forward(
...
guidance_scale=case.guidance_scale,
- seed=WAN21_LPIPS_SEED,
+ seed=case.seed,
)Also applies to: 318-366
🤖 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_wan.py` around
lines 80 - 107, Add a seed field to WanModelSpec and WanAccuracyCase, assign
WAN21_LPIPS_SEED or WAN22_LPIPS_SEED in each WAN_MODEL_SPECS entry, and
propagate it when constructing WAN_ACCURACY_CASES. Update
_generate_wan_feature_video to use the case’s seed instead of hardcoding
WAN21_LPIPS_SEED, preserving the correct seed for both WAN 2.1 and WAN 2.2
cases.
3f348bc to
ef82fa2
Compare
- split single-device accuracy tests into model-specific modules - add fixed goldens for FP8 blockwise, NVFP4, and CUDA graph - register supported feature cases in B200 L0 - record measured test durations Signed-off-by: Zhaoyuan He <zhaoyuanh@nvidia.com>
ef82fa2 to
a9459ed
Compare
5 participants
Description
This PR introduces a unified single-device accuracy regression framework for TensorRT-LLM VisualGen features.
It replaces the monolithic
test_visual_gen.pywith model-specific test modules and adds isolated accuracy coverage for:Each feature test enables exactly one feature and compares the generated output against a fixed golden produced with that same feature configuration. This detects changes in feature behavior across future code revisions instead of measuring the expected difference between a feature and an eager baseline.
TeaCache, Cache-DiT, VSA, and explicit
torch.compileprofiles are intentionally left for follow-up PRs.Motivation
VisualGen supports multiple model-specific optimizations, but previously lacked a consistent framework for detecting accuracy regressions caused by an individual feature.
The framework added by this PR provides:
FeatureConfigStateTest organization
The former monolithic test suite is split into:
test_visual_gen_flux.pytest_visual_gen_qwen_image.pytest_visual_gen_cosmos3.pytest_visual_gen_ltx2.pytest_visual_gen_wan.pyShared functionality is consolidated in:
visual_gen_test_utils.pyconftest.pyThe shared utilities cover feature configuration, runtime assertions, deterministic LPIPS evaluation, golden-media extraction, video handling, VBench support, and common WAN constants.
Existing multi-GPU test behavior is unchanged. Its imports are updated mechanically to use the shared utility module.
Accuracy-test design
For each feature profile, the test:
VisualGenArgsfrom a normalizedFeatureConfigState.All new feature tests keep
torch_compile=False, matching the existing upstream baseline test configuration.Existing baseline tests continue to use the upstream baseline JSON and media artifacts. Baseline outputs are not duplicated or regenerated by this PR.
Golden-output policy
This PR adds one fixed golden output for every model/feature profile.
Each golden JSON records:
The generation parameters match the corresponding existing upstream baseline. The only intentional configuration difference is the single feature under test.
Because the current implementation produced these fixed feature goldens, the validated LPIPS is
0.000000. Future numerical or behavioral changes will produce a nonzero LPIPS and fail if they exceed the0.05threshold.Feature coverage
Cosmos3-Nano CUDA graph is not included because it is not currently supported as an isolated feature profile.
This results in 20 new feature accuracy cases.
CI integration
All 20 feature cases are registered in the B200 L0 suite.
Existing VisualGen test references in the other test lists and waivers are updated to point to the new model-specific modules. No multi-GPU feature-matrix expansion is included.
LTX-2 feature tests do not use an explicit per-test timeout. Their measured durations are recorded in
.test_durationsfor CI scheduling.Test Coverage
All 20 single-feature cases passed on B200 with LPIPS
0.000000.Additional validation:
git diff --check: passedOut of scope
The following features are deferred to follow-up PRs so their accuracy policies and goldens can be reviewed independently:
torch.compileprofilesPR Checklist
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.