Improve PyTorch XDit benchmark workflow - #184
Open
speriaswamy-amd wants to merge 16 commits into
Open
Conversation
Remove the stale README reference to a deleted cluster example and redact WAN HF tokens in pytest output so the documented workflow stays accurate without exposing secrets. Co-authored-by: Cursor <cursoragent@cursor.com>
Add inline config comments so users know the sample validation metrics are cluster-specific placeholders that should be tuned before treating them as pass/fail thresholds. Co-authored-by: Cursor <cursoragent@cursor.com>
…anup-bugfixes # Conflicts: # cvs/lib/parallel/pssh.py
PytorchXditWan22Benchmarks and PytorchXditFlux1DevBenchmarks used extra="forbid", rejecting the _expected_results_comment key both shipped sample configs already include, so the configs failed schema validation out of the box. Switch to extra="allow", matching the existing PreflightConfigFile precedent for the same purpose. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
hf download stores blobs under blobs/ and exposes them via symlinks in the snapshot directory. wan_hf_snapshot_offline_check_commands() ran find without -L, so it stat'd the symlinks themselves (a few bytes) rather than their targets, and every size check reported MISSING even against a fully cached snapshot (0/6 shards matched without -L, 6/6 with it, confirmed on a real MI300X node). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ixed wall clock Both benchmark tests called exec_cmd_list(docker_cmds, timeout=1800), a hard 30-minute wall-clock cap. A real MI300X WAN 2.2 run legitimately takes ~45-60 minutes end to end (model load/compile plus 5 benchmark steps) while continuously producing output, so the fixed timeout killed healthy runs. Switch both call sites to inactivity_timeout, which only fires when output actually stalls. Threading inactivity_timeout through actually required fixing it at every layer these tests can resolve to: - MultiProcessPssh.exec_cmd_list() had no inactivity_timeout parameter at all (only main's Pssh.exec_cmd_list() gained one); add it and pass it through both the direct single-process path and the sharded worker path. - LocalPssh, the inline drop-in used by both test modules when the SSH target resolves to the local machine, had no timeout mechanism beyond subprocess.run(timeout=...). Give it a real per-line inactivity timeout via Popen + a reader thread + a Queue, mirroring Pssh's "resets on every output line" semantics. The 1800s value itself comes from a live diagnostic run: the longest silent gap observed between checkpoint-shard loading and the next log line (torch.compile of the 14B WanModel, 8 ranks) was ~323s in a clean run, but an earlier real run failed a 901s timeout, so 1800s is set with margin above the worst observed real failure point rather than just the clean-run sample. Also stop leaking the benchmark container on failure: kill it in the except block before fail_test(), mirroring the cleanup already done in test_cleanup_stale_containers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…VT-88) max_avg_total_time_s for mi300x was 12.0s, off by well over an order of magnitude from real hardware behavior (AIMVT-88's own reporter log shows 74.47s; this session's clean live measurement on MI300X with compile: true averaged 161.03s/step across 5 steps). Set it to 200.0s, ~25% margin above the measured average. auto and mi355 are left untouched: no live run was possible on those targets this session, and AIMVT-88's reported figure isn't self-consistent enough with either to derive a number from. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e HF cache (AIMVT-81/AIMVT-130) hf_home is resolved and staged independently per node; nothing in the setup steps warned that a per-node-local HOME on a multi-node cluster causes every node to redundantly download its own full model copy (AIMVT-81). Add an explicit callout to point hf_home at cluster-shared storage for multi-node runs, addressing AIMVT-130's ask for the exact steps needed to load and stage HF models. Also correct the WAN 2.2 storage requirement: the doc claimed ~40GB for model cache, but a real hf download of Wan-AI/Wan2.2-I2V-A14B measured 118GB. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
get_model_from_rocm_smi_output() only matched literal "MI300X"/"MI325"/ "MI350"/"MI355" substrings and fell back to 'mi300x' otherwise. Live testing on a real MI350 node (Ruby, driver 6.14.14) showed rocm-smi -a never emits any of those strings on this hardware/driver combination — only "Device ID: 0x75a0" and "GFX Version: gfx950" — so every pytorch_xdit and training/inference suite using this function silently mislabeled MI350 runs as mi300x and validated against the wrong threshold. Add a Device ID fallback for the confirmed MI350 ID (0x75a0). MI325/MI355 device IDs are not yet confirmed on real hardware, so they are left as marketing-name-only matches for now.
With the mi300x_pytorch_xdit_wan22_14b_single.json and mi300x_pytorch_xdit_flux1_dev_single.json. Both WAN22 and FLUX suites were live-verified on two separate MI350 (Ruby) nodes after the get_model_from_rocm_smi_output fix, confirming "Detected GPU type: mi350" in the logs on real hardware. Prior to this, MI350 runs had no dedicated threshold key and silently fell back to the 'auto' placeholder (15.0s for WAN22), which is far too tight for a real run and produced a false FAIL despite the benchmark performing normally (measured avg 86.10s-90.99s across two nodes). Add a measured mi350 key: 120.0s (~30% margin over the higher observed value), mirroring the mi300x threshold's own margin methodology (AIMVT-88). FLUX had never been run live before this effort (no HF token previously available). Measured avg pipe_time was 0.92s, consistent across two separate runs. Add a measured mi350 key: 2.0s (~2x margin), in place of relying on the unvalidated auto/mi300x placeholders.
Every test-category directory under cvs/tests/ except pytorch_xdit and
a handful of others (preflight, benchmark, anc) was missing __init__.py,
so setuptools' find_packages(where="cvs") silently excluded them from
setup.py's declared packages list. A normal `make install` (sdist-based,
non-editable) still worked by accident, since setuptools copies the
physical files regardless and importlib.resources.files() falls back to
Python's implicit namespace-package resolution for the missing parents.
Editable installs (pip install -e .) don't get that fallback: PEP 660's
finder maps only explicitly declared packages, so any package under an
undeclared parent resolves with __spec__.origin=None, and
cvs/cli_plugins/list_plugin.py's get_test_file() (which calls
importlib.resources.files()) fails with "expected str, bytes or
os.PathLike object, not NoneType" for every suite except pytorch_xdit.
Add the missing __init__.py to all 15 affected directories so
find_packages() declares them properly under both install modes.
This also let pylint's logging checker see cvs/tests/ibperf/
ib_perf_bw_test.py for the first time, surfacing a genuine latent bug:
log.info('%%%%%%%%% ib_bw_dict %%%%%%%%%%') has an odd count of '%'
characters, leaving a dangling format specifier; replaced the percent-
sign banner with a plain '=' banner.
…rement Measured avg 1.39s/pipe over 25 repetitions on MI300X (OCI, use_torch_compile: true), replacing the unvalidated 8.0s placeholder with 3.0s (~2x margin, consistent with the mi350 threshold's margin convention).
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
1 participant
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.
Summary
Test plan
make fmtmake lint.test_venv/bin/python run_all_unittests.pyCVS=\".test_venv/bin/cvs\" ./test_cli.shMade with Cursor