[https://nvbugs/6272644][fix] Stabilize and unwaive multi-GPU VisualGen LPIPS tests#15730
[https://nvbugs/6272644][fix] Stabilize and unwaive multi-GPU VisualGen LPIPS tests#15730chang-l wants to merge 1 commit into
Conversation
|
/bot run |
chang-l
requested review from
NVShreyas,
yibinl-nvidia and
zhenhuaw-me
and removed request for
yibinl-nvidia
|
/bot run |
|
PR_Github #56472 [ run ] triggered by Bot. Commit: |
|
PR_Github #56472 [ run ] completed with state |
📝 WalkthroughWalkthroughAdds FA4 attention backend and ChangesVisualGen FA4 fully-eager LPIPS evaluation
DGX B200 SLURM CI stage registration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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: 1
🤖 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/conftest.py`:
- Line 26: The test setup currently installs diffusers with a lower-bound
version spec, which can let future resolver changes drift from the recorded
golden. Update the installation in conftest.py to pin diffusers to the exact
version used by the golden sidecar (the recorded 0.38.0) so the LPIPS reference
stays stable; make the change in the llm_venv.run_cmd setup where the dependency
is installed.
🪄 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: 31d6d799-daef-4a99-a152-3b9521d36f4c
⛔ 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 (7)
jenkins/L0_Test.groovytests/integration/defs/examples/visual_gen/conftest.pytests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan22_t2v_fa4_fully_eager_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/golden/visual_gen_lpips/wan22_t2v_lpips_golden_video.jsontests/integration/defs/examples/visual_gen/test_visual_gen.pytests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.pytests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
- tests/integration/test_lists/waives.txt
chang-l
force-pushed
the
dev-visgen-multigpu-lpips
branch
from
fe689ab to
db18159
Compare
|
/bot run |
2 similar comments
|
/bot run |
|
/bot run |
|
PR_Github #56532 [ run ] triggered by Bot. Commit: |
|
PR_Github #56532 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #56639 [ run ] triggered by Bot. Commit: |
|
PR_Github #56639 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #56692 [ run ] triggered by Bot. Commit: |
|
PR_Github #56692 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #58315 [ run ] triggered by Bot. Commit: |
|
PR_Github #58315 [ run ] completed with state |
|
/bot run --disable-fail-fast --extra-stage "DGX_B200-4_GPUs-PyTorch-Post-Merge-1, DGX_B200-4_GPUs-PyTorch-Post-Merge-2, DGX_B200-4_GPUs-PyTorch-Post-Merge-3, DGX_B200-4_GPUs-PyTorch-Post-Merge-4, DGX_B200-PyTorch-Post-Merge-1, DGX_B200-PyTorch-Post-Merge-2" |
|
PR_Github #58321 [ run ] triggered by Bot. Commit: |
|
PR_Github #58321 [ run ] completed with state |
chang-l
force-pushed
the
dev-visgen-multigpu-lpips
branch
from
5518466 to
3c570f1
Compare
|
Note for reviewers: squashed the branch to a single commit (tree is byte-identical to the previous head |
|
/bot run --disable-fail-fast --extra-stage "DGX_B200-4_GPUs-PyTorch-Post-Merge-1, DGX_B200-4_GPUs-PyTorch-Post-Merge-2, DGX_B200-4_GPUs-PyTorch-Post-Merge-3, DGX_B200-4_GPUs-PyTorch-Post-Merge-4, DGX_B200-PyTorch-Post-Merge-1, DGX_B200-PyTorch-Post-Merge-2" |
|
PR_Github #58324 [ run ] triggered by Bot. Commit: |
There was a problem hiding this comment.
If this is required? I don't get it why we need to "hack the path" for multi-gpu tests, does LLM test do the same?
chang-l
force-pushed
the
dev-visgen-multigpu-lpips
branch
from
3c570f1 to
8c2f5d9
Compare
|
/bot run |
|
PR_Github #58416 [ run ] triggered by Bot. Commit: |
|
PR_Github #58324 [ run ] completed with state
|
|
PR_Github #58416 [ run ] completed with state
|
|
/bot run |
|
PR_Github #58423 [ run ] triggered by Bot. Commit: |
|
PR_Github #58423 [ run ] completed with state
|
chang-l
force-pushed
the
dev-visgen-multigpu-lpips
branch
from
8c2f5d9 to
a9774ce
Compare
|
/bot run |
|
PR_Github #58532 [ run ] triggered by Bot. Commit: |
|
PR_Github #58532 [ run ] completed with state
|
|
/bot run |
|
PR_Github #58600 [ run ] triggered by Bot. Commit: |
…en LPIPS tests
Compare every Wan 2.2 multi-GPU LPIPS variant against a dedicated
single-GPU FA4 fully-eager reference so the comparison isolates the
parallel decomposition: disable compile via TorchCompileConfig and a
scoped torch.compiler.set_stance("force_eager"), enable deterministic
algorithms, and restore all prior state afterwards.
- add the one-GPU FA4 fully-eager Wan 2.2 golden, regenerated under the
26.04 staging release image (bit-identical double generation; sidecar
records image digest, versions, and source commit)
- pin diffusers==0.38.0 in requirements-dev.txt so the test environment
matches the version the goldens were recorded with
- strictly validate the spawn-worker tensorrt_llm site directory both
before spawning and inside each worker
- share the VisualGen dependency fixture between single- and multi-GPU
test modules; defer defs imports so spawn workers survive re-import
- unwaive all eight multi-GPU/TP LPIPS variants and move the two
eight-worker variants to B200 8-GPU pre-merge coverage
Branch squashed to a single commit: earlier revisions of the golden
media zip existed only in fork LFS storage and broke the CI GitHubSync
mirror push (pre-receive: LFS objects missing).
Signed-off-by: Chang Liu <9713593+chang-l@users.noreply.github.com>
chang-l
force-pushed
the
dev-visgen-multigpu-lpips
branch
from
a9774ce to
fc9b55f
Compare
|
PR_Github #58600 [ run ] completed with state
|
|
/bot run |
|
PR_Github #58683 [ run ] triggered by Bot. Commit: |
5 participants
Summary
TorchCompileConfig(enable=False)and a scopedtorch.compiler.set_stance("force_eager")0.25thresholdMotivation
PR #15564 enabled single-GPU VisualGen LPIPS protection. The multi-GPU Wan tests remained waived because their reference execution policy was not encoded independently from compile behavior, so compile drift could obscure the accuracy delta introduced by the parallel strategy itself.
This change makes the comparison single-delta: the reference and distributed candidates use the same FA4 backend and fully-eager policy. The remaining LPIPS difference therefore measures the selected parallel decomposition rather than a backend or compilation change.
Related NVBugs:
Implementation
CUBLAS_WORKSPACE_CONFIGstatediffusers==0.38.0inrequirements-dev.txtso the test environment matches the version the goldens were recorded with (the current nightly release image already ships 0.39.0)tensorrt_llmsite directory (must contain the installed package with compiled bindings) both before spawning and inside each workerB200 validation
Re-validated after the rebase, on an 8×B200 node, against the FA4 fully-eager reference regenerated in the current staging
main-x86_64release image:ulysses4cfg2_ulysses2attn2d_2x2cfg2_ulysses2_attn2d_2x1attn2d_2x2_ulysses2tp2cfg2_tp2tp2_ulysses2All eight results are below the unchanged
0.25threshold. Two independent reference generations produced bit-identical MP4s (matching SHA-256), and no Dynamo compile counters were populated. The single-GPUtest_wan22_t2v_lpips_against_goldenpasses in the same environment (LPIPS 0.040069 < 0.05 against main's golden). The old (26.02-era) and new (26.04) FA4 references differ by LPIPS 0.0397 — the expected container-drift magnitude that motivated the regeneration.The two eight-worker cases completed in 98.0 and 96.6 seconds, including distributed generation, MP4 encoding, and LPIPS evaluation.
Validation used the staging
main-x86_64release image pinned to digestsha256:0783d275ffe7efc3b6093dd6c7e823cd703eae973e462c8e1e4f7ca7dad60d64(26.04 stack, TensorRT-LLM 1.3.0rc21 built from maina0c406ff88), withdiffusers==0.38.0installed per the requirements-dev pin.Checks
git diff --check