[TRTLLM-13304][test] Add VisualGen multi-node E2E and external-launch unit coverage - #17271
Open
yingguo-trt wants to merge 22 commits into
Open
[TRTLLM-13304][test] Add VisualGen multi-node E2E and external-launch unit coverage#17271yingguo-trt wants to merge 22 commits into
yingguo-trt wants to merge 22 commits into
Conversation
yingguo-trt
requested review from
NVShreyas,
QiJune,
StanleySun639,
mzweilz,
niukuo,
schetlur-nv and
yibinl-nvidia
August 5, 2026 02:22
Collaborator
Author
|
/bot run --stage-list "DGX_B200-16_GPUs-2_Nodes-PyTorch-VisualGen-Post-Merge-1" |
Contributor
|
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:
WalkthroughAdded SLURM-based two-node VisualGen testing for B200 systems. The change adds rank coordination, nested ChangesVisualGen multinode execution
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Jenkins
participant ParentPytest
participant srun
participant VisualGenRanks
participant GoldenVideo
Jenkins->>ParentPytest: start B200 two-node test
ParentPytest->>srun: launch sixteen pytest tasks
srun->>VisualGenRanks: provide SLURM rank and node metadata
VisualGenRanks->>GoldenVideo: generate and evaluate rank-zero output
VisualGenRanks-->>ParentPytest: return rank test results
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
jenkins/L0_Test.groovy (1)
1649-1650: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMatch the VisualGen multinode mode against a set instead of one exact name.
visualGenMultinodeSlurmModecomparestestListto a single literal. A second VisualGen multinode list, for example a B300 variant, would silently take thetrtllm-llmapi-launchpath andgetNodeArgs. The Python parent then fails with its explicittrtllm-llmapi-launchmessage, so the fault is diagnosable but the wiring is easy to miss.Declare the mode as a named set so a new list only has to be added in one place.
♻️ Proposed refactor
Add a field near the other pipeline literals:
// Test lists whose parent process shells out to its own srun step, so the // trtllm-llmapi-launch wrapper must not strip the SLURM rank environment. `@Field` def VISUAL_GEN_MULTINODE_TEST_LISTS = ["l0_b200_visual_gen_multinode"] as SetThen use it at both sites:
- def visualGenMultinodeSlurmMode = testList == "l0_b200_visual_gen_multinode" + def visualGenMultinodeSlurmMode = VISUAL_GEN_MULTINODE_TEST_LISTS.contains(testList) if (nodeCount > 1 && !visualGenMultinodeSlurmMode) {Also applies to: 1694-1696
🤖 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 `@jenkins/L0_Test.groovy` around lines 1649 - 1650, Replace the single-name comparison in the visualGenMultinodeSlurmMode logic with a named `@Field` set of supported VisualGen multinode test lists, initialized with the existing list. Use membership checks against this set at both referenced sites, so future variants can be added centrally while preserving the current behavior.tests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.py (1)
253-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRestore the mutated launch environment after the rank test.
_ensure_slurm_external_launch_envwritesMASTER_ADDRandMASTER_PORTintoos.environand removesRANK,WORLD_SIZE, andLOCAL_RANK. The changes persist for the rest of the pytest process. Inside a SLURM allocationSLURM_PROCIDandSLURM_NTASKSstay set, so any later test in the same process would also take the VisualGen external-launch branch.The current test list for this stage contains only this test, so nothing breaks today. Convert the helper into a
monkeypatch-based fixture or a context manager so the behavior stays correct if the stage later gains more tests.🤖 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_multi_gpu.py` around lines 253 - 261, The helper _ensure_slurm_external_launch_env mutates os.environ without restoring it, allowing later tests to inherit the external-launch state. Convert _ensure_slurm_external_launch_env to use pytest’s monkeypatch fixture or a context manager, and apply temporary environment updates/removals through that mechanism so all changes are automatically reverted after the rank test.
🤖 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 `@jenkins/scripts/slurm_run.sh`:
- Around line 66-72: Update the marker wait loop in the rank-0 branch to enforce
a deadline shorter than the partition walltime, and fail explicitly when it
expires. Before failing, report the observed marker set (or equivalent marker
details) and the expected node count so the missing node is diagnosable;
preserve the existing wait behavior while the deadline has not been reached.
In `@tests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.py`:
- Around line 418-434: Replace the bare subprocess.run call in the multi-node
VisualGen test with the canonical popen helper and manage it through communicate
with the existing timeout. Import popen and cleanup_process_tree from
trt_test_alternative using the established integration-test import style; on
subprocess.TimeoutExpired, invoke cleanup_process_tree(proc) before failing with
the existing timeout message and captured output behavior.
- Around line 442-451: Update the output guard after the VisualGen SLURM run to
parse the pytest summary line rather than searching raw output substrings. Use
the module’s imports and a regular-expression match to identify the final
summary, rejecting runs with skipped tests or no collected tests while relying
on srun’s non-zero status for failures; do not treat incidental log text or a
single rank’s “passed” message as proof of success.
---
Nitpick comments:
In `@jenkins/L0_Test.groovy`:
- Around line 1649-1650: Replace the single-name comparison in the
visualGenMultinodeSlurmMode logic with a named `@Field` set of supported VisualGen
multinode test lists, initialized with the existing list. Use membership checks
against this set at both referenced sites, so future variants can be added
centrally while preserving the current behavior.
In `@tests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.py`:
- Around line 253-261: The helper _ensure_slurm_external_launch_env mutates
os.environ without restoring it, allowing later tests to inherit the
external-launch state. Convert _ensure_slurm_external_launch_env to use pytest’s
monkeypatch fixture or a context manager, and apply temporary environment
updates/removals through that mechanism so all changes are automatically
reverted after the rank test.
🪄 Autofix
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: 8a14eb7a-733c-47e5-947b-d5ab66a16472
📒 Files selected for processing (7)
jenkins/L0_MergeRequest.groovyjenkins/L0_Test.groovyjenkins/scripts/slurm_run.shtests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.pytests/integration/test_lists/test-db/l0_b200_visual_gen_multinode.ymltests/unittest/_torch/visual_gen/multi_gpu/test_visual_gen_mapping.pytests/unittest/_torch/visual_gen/multi_gpu/test_visual_gen_multinode.py
Collaborator
|
PR_Github #63904 [ run ] triggered by Bot. Commit: |
BowenFu
reviewed
Aug 5, 2026
Add QA coverage for multi-node VisualGen (dev epic TRTLLM-11407): - SLURM 2-node x 8-GPU end-to-end LPIPS test for cfg2_attn2d_2x2_ulysses2, exercising the real _detect_external_launch SLURM path; parent pre-checks the checkpoint and validates srun output to avoid an all-skip false pass. - world_size=16 rank/group unit tests (attn2d and ring) using a fake DeviceMesh, extending coverage beyond the 8-GPU single-node lane. - Register the multi-node case in qa/llm_function_multinode.txt. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
…-llmapi-launch The wan22 multinode SLURM case needs real SLURM_* env for rank detection, but trtllm-llmapi-launch strips SLURM_* before user code. Downgrade the hard pytest.fail to pytest.skip so the case skips gracefully under that launcher, consistent with its other environment prerequisite checks, instead of failing the pipeline. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
…RM test VisualGen multinode uses torch.distributed + NCCL (TLLM_DISABLE_MPI=1) and must NOT be wrapped with trtllm-llmapi-launch, which strips SLURM_* env. Add a dedicated l0_b200_visual_gen_multinode list + 2-node/16-GPU post-merge stage that launches without the MPI wrapper: one task per node sets up, then rank 0 runs the parent pytest with SLURM rank env stripped so the test self-orchestrates its own 16-rank srun. WIP pending dev confirmation of the intended VisualGen multinode launch recipe (TRTLLM-11407 still in progress). Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
…wiring The external-launch branch of VisualGen.__init__ (world-size validation, rank!=0 pure-worker exit) and the rank-0 DiffusionRemoteClient wiring (ZMQ connect addresses must use the launcher's master_addr, no local mp.Process spawn, own worker in a daemon thread with in_client_process=True) had no unit coverage — a wrong connect address keeps every single-node test green and only fails on a real 2-node run. Add CPU-only mocked tests following the existing patterns in test_visual_gen_multinode.py. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
ParallelConfig is only imported lazily inside _parallel_config() (kept lazy for mp.spawn sys.path ordering), so the bare ParallelConfig reference in _run_wan22_multinode_slurm_rank raised NameError on every SLURM rank before VisualGen was even constructed. Use the _parallel_config helper like the other call sites. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
… deps in multinode case Two blockers in the VisualGen multinode SLURM case: - MODULES_AVAILABLE was referenced at the top of the SLURM rank path but never defined in this file (ruff F821), so every rank raised NameError before constructing VisualGen. Drop the stale guard copied from the unittest harness; module-level imports already fail loudly. - The multinode nodeid never requested _visual_gen_deps, so a clean node lacked av/ffmpeg for rank 0's MP4 save and video LPIPS eval. Resolve the fixture parent-side only via request.getfixturevalue: rank 0 shares node 0's venv with the parent, remote workers never touch media deps, and the bare child pytest run has no llm_venv harness (and 16 ranks racing pip/apt would be unsafe). Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
Test-list TIMEOUT is in minutes (test_list_parser.py multiplies by 60), so TIMEOUT (3600) on the multinode nodeid meant 60 hours — a hung run would pin a 2-node 16xB200 allocation for days. Set it to 90 minutes. The QA multinode list entry carries no TIMEOUT marker and falls back to the pipeline's outer --timeout=3600s, which would fire before the inner srun subprocess timeout (same 3600s) and lose its captured output. Lower the inner default to 3300s so the srun timeout with output always fires first. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
…dy unit tests The multinode SLURM case has exactly one parallel variant, so drop the parametrization in favor of a WAN22_LPIPS_MULTINODE_PARALLEL constant and sync the nodeid in the dedicated L0 stage. Remove the QA multinode list entry: that pipeline wraps pytest with trtllm-llmapi-launch, which strips SLURM_* env before user code, so the case would always hard-fail there — the dedicated post-merge SLURM stage remains the single consumer. Compress assertions and drop banner comments in the multinode unit tests. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
…node list The QA cluster pipeline (llm_test_cluster) runs this list with direct srun (-N2 --ntasks-per-node=8 = 16 ranks) and already opts multinode_slurm cases out of the trtllm-llmapi-launch wrapper, so the SLURM rank environment reaches the test and every rank takes the external-launch rank path. The earlier removal assumed the wrapper always applies; re-add the entry with the de-parametrized nodeid. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
pytest-threadleak flags three daemon threads after the multinode case: InductorSubproc / subproc_worker_timer (unconditional @torch.compile spawns the inductor worker pool; same root cause as nvbugs/6215688) and rank 0's run_diffusion_worker in-client thread (shutdown()'s bounded join can be outlasted by 16-rank dist teardown). Reuse the existing fix pattern from test_visual_gen.py: disable the inductor quiesce timer up front and call _cleanup_cuda() (shutdown_compile_workers) in the shared finally so every rank cleans up, and explicitly join rank 0's worker thread after shutdown. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
…aseline The multinode case runs the FA4 backend fully-eager like the single-node multi-GPU cases, but compared its output against the default-path golden, so the LPIPS score was not comparable. Use WAN22_MULTI_GPU_LPIPS_GOLDEN_VIDEO and wrap the run in _lpips_deterministic_algorithms(fully_eager=True) to match _wan22_lpips_distributed_worker, which the multinode path bypasses by calling VisualGen directly. Rename the test for consistency with the other LPIPS nodeids and drop the QA multinode list entry: the dedicated L0 post-merge stage already covers this case. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
…inode list The QA cluster pipeline (llm_test_cluster) runs this list with direct srun and skips the trtllm-llmapi-launch wrapper for nodeids containing multinode_slurm, so the SLURM rank environment reaches the test. Register the case so it can be exercised through the QA pipeline before the PR lands. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
VisualGen.shutdown() drops its executor reference (self.executor = None) after tearing down DiffusionRemoteClient, so reading visual_gen.executor._ext_worker_thread afterwards always resolved to None and the explicit join never ran. pytest-threadleak still flagged the leaked run_diffusion_worker thread. Grab the thread reference before calling shutdown(). The client's own join uses WORKER_TIMEOUT = 2s, which 16-rank dist.destroy_process_group() plus pipeline cleanup routinely outlasts, so the generous follow-up join is what actually lets the thread finish. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
The parent-side _visual_gen_deps call never reached the ranks: each srun step starts a fresh container from the image, so the parent's apt-get install ffmpeg lives only in its own writable layer. rank 0 then hit "MP4 format requires ffmpeg to be installed" when encoding the generated video, because _check_ffmpeg_available() probes the ffmpeg CLI on PATH. Move the fixture call into the rank path, gated on rank == 0 (the only rank that encodes a video), so the deps land in the process that needs them without 16 tasks racing the same apt lock. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
…ode list The case is covered by the dedicated L0 post-merge stage (DGX_B200-16_GPUs-2_Nodes-PyTorch-VisualGen-Post-Merge-1 / l0_b200_visual_gen_multinode), which drives srun directly via jenkins/scripts/slurm_run.sh. The QA cluster pipeline wraps every multinode nodeid in trtllm-llmapi-launch, which strips SLURM_* before user code and is incompatible with VisualGen external-launch mode, so registering it there would need a matching trt_jenkins change to be useful. Keep the L0 registration as the single home for this case. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
All 22 existing multi-node test-db lists spell it multi_nodes (l0_gb200_multi_nodes.yml, l0_b200_multi_nodes_perf_sanity_*.yml, ...); this file was the only one using "multinode". The difference is not cosmetic: CODEOWNERS line 61 routes tests/integration/test_lists/test-db/*multi_node* to @NVIDIA/trt-llm-multi-gpu-ci-review, and "multinode" does not match that glob, so the new 16-GPU 2-node list silently bypassed that review. Rename the file and its list key, and update the three references in L0_Test.groovy and L0_MergeRequest.groovy. The stage name DGX_B200-16_GPUs-2_Nodes-PyTorch-VisualGen-Post-Merge-1 already follows the existing *-2_Nodes-* convention and is unchanged. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
Ring is already covered three ways in this file: the ring+attn2d rejection case, the cfg2/ring2/ulysses2 fiber-flattening case, and TestMultiGPU::test_cfg2_ring2_ulysses2, which validates ring ranks and groups against a real DeviceMesh. Going from ring_size=2 at world_size 8 to ring_size=4 at world_size 16 exercises the same modulo arithmetic and adds no new failure mode. Ring is also not the topology TRTLLM-13304 covers: the multi-node E2E runs cfg x attn2d x ulysses. Keep only the world_size-16 attn2d case, which mirrors that E2E and has no existing counterpart (the current tests top out at 8 GPUs and three dimensions). Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
- slurm_run.sh: bound the rank-0 install-marker wait with a deadline (TRTLLM_VISUAL_GEN_MULTINODE_INSTALL_TIMEOUT, default 3600s) and print the markers found, instead of looping forever if a node never reports. - Parent path: drive srun through defs.trt_test_alternative.popen so the whole process tree is killed on timeout; plain subprocess.run only reaps the local srun and can strand remote tasks. popen also refills TimeoutExpired.output, so the failure message keeps the srun log. - Parent path: validate the per-rank pytest summaries that srun -l emits (exactly one "1 passed" per rank, ANSI stripped) rather than searching the whole log for "passed"/"skipped", which any log line could satisfy. - Cross-reference WAN22_LPIPS_MULTINODE_PARALLEL and the 16-rank mapping unit test in both directions; nothing enforces that they agree, and a drifted unit test would silently guard a topology nobody runs. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
…uards
Scope: drop both unit-test additions. This PR does not modify
VisualGenMapping or the external-launch code they guard, real multi-GPU
mapping tests already cover attn2d/ulysses grouping against a live
DeviceMesh, and the fake DeviceMesh re-implemented init_device_mesh's
row-major rule rather than verifying it. Both files are now byte-identical
to main; the PR is the multi-node E2E plus its CI registration.
Fixes:
- Guard on TLLM_SPAWN_PROXY_PROCESS alone. trtllm-llmapi-launch strips
SLURM_* at any world size, so gating on tllm_mpi_size >= 16 let the
stripped-env case fall through to a "no SLURM allocation" skip that
points at the wrong cause. Drops the now-unused helper.
- Scope the install markers to ${SLURM_JOB_ID}. runLLMTestlistOnSlurm
retries on infra failure and can reuse jobWorkspace; stale node_* files
would let rank 0 stop waiting before this attempt's nodes finish
installing, resurfacing the missing-ffmpeg failure intermittently.
Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
Contributor
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 (2)
jenkins/scripts/slurm_run.sh (2)
1-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the NVIDIA copyright header and update the year.
This modified shell source has no NVIDIA copyright header. Add the repository-standard header at the top and set its copyright year to 2026.
As per coding guidelines:
**/*requires the NVIDIA copyright header on all new files and the latest meaningful modification year on modified files.🤖 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 `@jenkins/scripts/slurm_run.sh` around lines 1 - 5, Add the repository-standard NVIDIA copyright header at the beginning of slurm_run.sh, before the shebang, and set its copyright year to 2026; leave the existing error-handling commands unchanged.Source: Coding guidelines
87-87: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClear inherited MPI variables before the VisualGen parent pytest.
The parent launches nested
srun --overlap --export=ALL, so inheritedPMI*,PMIX*,MPI*, andOMPI*variables can reach the child and trigger MPI initialization. Remove these variables, but preserve theSLURM_*metadata required by the parent to detect the allocation and launch the child.🤖 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 `@jenkins/scripts/slurm_run.sh` at line 87, Update the pytestCommand environment cleanup to unset inherited PMI*, PMIX*, MPI*, and OMPI* variables before launching the VisualGen parent pytest, while retaining the existing SLURM_* variables needed for allocation detection and child launch.
🤖 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 `@jenkins/scripts/slurm_run.sh`:
- Line 63: Add the required NVIDIA copyright header at the top of
jenkins/scripts/slurm_run.sh, using the year of the latest meaningful
modification, and remove the redundant local assignment for jobWorkspace while
preserving its exported value for the container.
---
Outside diff comments:
In `@jenkins/scripts/slurm_run.sh`:
- Around line 1-5: Add the repository-standard NVIDIA copyright header at the
beginning of slurm_run.sh, before the shebang, and set its copyright year to
2026; leave the existing error-handling commands unchanged.
- Line 87: Update the pytestCommand environment cleanup to unset inherited PMI*,
PMIX*, MPI*, and OMPI* variables before launching the VisualGen parent pytest,
while retaining the existing SLURM_* variables needed for allocation detection
and child launch.
🪄 Autofix
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: db8aaa17-1c24-473e-affe-671b30623266
📒 Files selected for processing (2)
jenkins/scripts/slurm_run.shtests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/integration/defs/examples/visual_gen/test_visual_gen_multi_gpu.py
… step The outer srun step for this stage runs only a shell script that starts pytest; the real 16-way fan-out is an inner srun the parent issues itself, and VisualGen never initializes MPI (TLLM_DISABLE_MPI=1 + torch.distributed). Bootstrapping PMIx there injects PMIX_* describing the 2-task outer step, and slurm_run.sh only unsets five SLURM_* names, so the inner srun --export=ALL hands those stale variables to all 16 child ranks. Pass --mpi=none rather than dropping the flag: with no --mpi, srun falls back to the cluster's MpiDefault, which is commonly pmix, so omitting it would not reliably disable the bootstrap. Other stages keep --mpi=pmix unchanged. Also trims the two verbose comments added in the previous commit to match the surrounding one-line style. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
The suffix guarded a problem that does not exist. runLLMTestlistWithSbatch derives jobWorkspace from a fresh UUID on every call (L0_Test.groovy:1490) and the infra-retry loop re-enters that function per attempt, so retries never share a marker directory. The submit script also rm -rf's everything outside the keep list immediately before sbatch (L0_Test.groovy:1968). The one path that does reuse a workspace -- "reuse an already-active job" -- reuses the same SLURM job, so SLURM_JOB_ID would be identical and the suffix would protect nothing. Restores the plain marker directory and drops the comment, which claimed infra retries reuse jobWorkspace and was simply wrong. The wrapper-guard fix and --mpi=none stay. Signed-off-by: Ying Guo <244492186+yingguo-trt@users.noreply.github.com>
yingguo-trt
force-pushed
the
yiguo/TRTLLM-13304-visualgen-multinode-qa
branch
from
August 5, 2026 03:42
b3078a9 to
9119291
Compare
Collaborator
Author
|
/bot run |
Collaborator
|
PR_Github #63926 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #63904 [ run ] completed with state
|
Collaborator
|
PR_Github #63926 [ run ] completed with state
|
Collaborator
Author
|
/bot run |
Collaborator
|
PR_Github #63945 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #63945 [ run ] completed with state
|
Collaborator
Author
|
/bot run --disable-fail-fast |
Collaborator
|
PR_Github #64017 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #64017 [ run ] completed with state
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Dev Engineer Review
QA Engineer Review
test_wan22_t2v_multinode_slurm_lpips_against_golden.tests/integration/test_lists/test-db/l0_b200_visual_gen_multi_nodes.yml.Description
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.