[zxd1997066]

Add BMG runner coverage for distributed tests, plus a weekly-only BMG suite using a temporary validation branch.

Changes:

• pull.yml: linux-distributed now runs xpu_distributed on both distributed and bmg-test runners.
• nightly_ondemand.yml: new xpu_distributed_bmg ut scope, only auto-added on the Saturday nightly-wheel schedule (30 16 * * 5); also usable via on-demand dispatch.
• linux-uttest/action.yml: new xpu_distributed_bmg step that clones the daisyden/distributed_2.13 branch inside the step and runs run_distributed_local.py.
• ut_result_check.sh: route xpu_distributed_bmg through the same result-parsing path as xpu_distributed.
• run_distributed.py: fix ZE_AFFINITY_MASK on BMG. BMG topology has no XL-linked GPU groups (all SYS), so the old logic collapsed to a single GPU index; now falls back to the first WORLD_SIZE GPUs when no group is found.

[chuanqi129]

@torchxpubot review

[torchxpubot]

Replying to this comment by @chuanqi129

VERDICT: Request changes — the pull.yml runner matrix makes both distributed jobs collide on the same artifact/summary identity (proven by this PR's own run: digest-mismatch errors, artifact deleted and overwritten, the two summaries disagreeing), and the weekly scope runs unreviewed code from a floating personal branch while mutating the installed torch in place.

Details

I have enough evidence. Writing the review.

PR Review: #5147

Summary

Adds BMG coverage for the distributed suite in pre-CI plus a new xpu_distributed_weekly scope that runs tests from a temporary branch, and makes run_distributed.py pick a GPU mask on BMG's all-SYS topology.

Correctness

.github/workflows/pull.yml:288 — the runner matrix collides on artifact identity, and this PR's own CI shows it.
_linux_ut.yml keys every artifact and every summary lookup on inputs.ut, never on the runner: Inductor-XPU-UT-Data-<pr>-<ut>-<run_id>-<run_attempt> (lines 74, 120, 269) and the summary's download pattern (line 148) / latest_dir find (line 160). With runner: [distributed, bmg-test] both calls pass ut: xpu_distributed, so both produce the same artifact name. In run 38230091075:

• download-artifact reported digest-mismatch: error in both summary jobs, and latest_dir came out empty in both (+ latest_dir= → .), i.e. the per-artifact directory the script expects never materialised.
• The distributed summary logged Artifact 'Inductor-XPU-UT-Data-5147-xpu_distributed-38230091075-1' (ID: 11743010239) deleted and re-uploaded its own (ID 11742071208) — the re-upload at line 269 has overwrite: true, so one runner's logs destroy the other's. The bmg-test summary then downloaded yet another ID (11742997339).
• Net result: linux-distributed (xpu_distributed, bmg-test) / summary passed and linux-distributed (xpu_distributed, distributed) / summary failed, even though both test-in-baremetal jobs passed. Which logs each summary grades is not deterministic, and the run's final artifact contains only one runner's data.

_linux_ut.yml already has the input for this: category is documented as "Optional artifact tag". Pass category: ${{ matrix.runner }} from the matrix (it also keeps the known-failure label scope per runner, since fetch_issues.sh adds skipped_bmg from inputs.runner), or give the two entries distinct ut labels. Also note this changes the check names from linux-distributed (xpu_distributed) to linux-distributed (xpu_distributed, <runner>); if any are listed in branch protection they need re-adding.

linux-distributed is red on this PR and the failures are not muted. The distributed summary reports two new failures: test_fsdp_core.py::TestAutogradXPU::test_unshard_params_as_tensors_xpu (300 s timeout) and test_functional_api.py::TestTraceableCollectives::test_all_gather_into_tensor_coalesced_device_xpu. Neither appears in the BMG run, and the PVC mask is unchanged by this PR (the XL8 groups still yield 0,1,2,3), so they look pre-existing/flaky rather than caused by the diff — but they have to be triaged and muted (or shown flaky with a re-run link) before this lands, otherwise a red linux-distributed becomes the merged baseline and hides the collision above.

test/xpu/run_distributed.py:51-53 — WORLD_SIZE default of 4 is the value that actually takes effect, and it is a magic number. The pre-CI path (run_distributed.py, non-weekly) never exports WORLD_SIZE, so the fallback always uses 4. On the bmg-test runner the environment already exposed ZE_AFFINITY_MASK=0,1,2,3,4,5,6,7 (8 GPUs, confirmed in the job log) and this code narrowed it to 0,1,2,3 — half the machine, with no log line saying a fallback happened or why 4. Derive the count from what is available (len(gpu_dict), or the inherited ZE_AFFINITY_MASK), or state the reason for 4 in the comment.

Same block — the fallback is not actually BMG-specific. The condition is "no multi-GPU group was found", which also matches a PVC/XL fabric that has degraded to all-SYS. Previously that case pinned to a single GPU and failed loudly; now it quietly runs collectives over non-XL-linked GPUs, which surfaces as slow or oddly-failing tests instead of a clear topology error. At minimum print that no XL group was found before overriding.

XPU-Specific Risks

.github/actions/linux-uttest/action.yml:193 — the weekly path overwrites the installed torch in place. shutil.copyfile('distributed/distributed_test.py', inspect.getsourcefile(m)) rewrites torch/testing/_internal/distributed/distributed_test.py inside site-packages, on a baremetal runner, with no restore. Every later job sharing that Python environment on that machine (upstream_distributed, op_ut, …) inherits the patched module. Scope it instead — e.g. prepend the branch's directory to PYTHONPATH, or restore the original file in a post/always step.

action.yml:192 — sudo rm -rf /tmp/torchinductor_jenkins/ hardcodes another user's cache path, needs root to do it, and carries no explanation of what it fixes. Set TORCHINDUCTOR_CACHE_DIR under the job workspace instead, or at least comment why this specific directory must be deleted.

action.yml:186-190 — undocumented runtime switches. USE_CCL_V2=1, BACKEND=xccl, TEMP_DIR=/tmp and a hardcoded WORLD_SIZE=4 (on an 8-GPU box) appear with no comment. USE_CCL_V2 in particular interacts with the recent libccl.so.1/so.2 workarounds (#5648, #5670) — say why this scope needs it and the others don't.

Dispatch & Registration

action.yml:174-179 — the weekly branch replaces the prepared checkout. rm -rf pytorch/third_party/torch-xpu-ops + git clone --depth 1 --branch daisyden/distributed_2.13 means (a) the torch_xpu_ops input to _linux_ut.yml is silently ignored for this scope, (b) the tests no longer correspond to the torch-xpu-ops the wheel was built from, and (c) CI executes code that is not reviewable in this repo, in a job whose environment carries GH_TOKEN, HF_TOKEN and HUGGING_FACE_HUB_TOKEN (_linux_ut.yml:40-44). --branch with no commit pin also floats: whoever pushes to that branch changes what weekly CI runs, and results are not reproducible. Pin a SHA, add a comment stating this is temporary with the tracking issue and removal condition, and land these tests in main behind the new scope as the actual fix.

nightly_ondemand.yml:174 (unchanged line) — the new scope's hardware is implicit. runner: ${{ (matrix.ut_name == 'xpu_distributed' || matrix.ut_name == 'upstream_distributed') && 'distributed' || 'bmg-test' }} sends xpu_distributed_weekly to bmg-test only by falling through the ternary. The single thing that makes this a BMG suite is invisible at the point where someone would edit that expression. Make it explicit, and reconsider the rename: xpu_distributed_weekly encodes cadence (it is also reachable by on-demand dispatch on any day) while dropping the only hint about hardware — the original xpu_distributed_bmg said what it was.

nightly_ondemand.yml:108-110 — the weekly wheel run loses the regular distributed suite. ut="${ut/xpu_distributed/xpu_distributed_weekly}" substitutes rather than appends, so the Saturday nightly-wheel run no longer covers xpu_distributed at all. The PR description says the scope is "only auto-added", which is not what this does — decide which you want and make description and code agree. Also, this re-tests the raw cron string that the block above has already turned into test_type; use [[ "${test_type}" == "wheel-weekly" ]] so the two don't drift when the schedule changes.

Known-failure muting won't apply to the new scope. The awk filter in _linux_ut.yml:168-173 selects known-issue case lines with $1 ~ ut_name, and auto-filed case lines are <category>,<class>,<test> with category == "xpu_distributed" (ut_collect_evidence.py:142). With ut_name == "xpu_distributed_weekly" nothing matches, so every skipped/skipped_bmg issue filed for distributed is ignored for this suite (the static list from issue 1624 still applies, since it is not filtered). Please confirm against a real weekly dispatch run that the suite isn't red purely from unmuted pre-existing failures.

Testing

The PR description carries no link to a successful run of the new weekly scope. Since xpu_distributed_weekly only fires on a cron, it will first execute unvalidated in production — trigger it once via workflow_dispatch and link the run.

Performance

Pre-CI now runs the full distributed suite on two machines for every PR (42 m on distributed, 53 m on bmg-test in this run). That is a deliberate cost increase; worth saying in the description that it is intended, and whether a label-gated or nightly-only BMG distributed run was considered instead.

Other

• action.yml:170-175: the test_scope if/else is a no-op — it equals inputs.ut_name in both branches. Use test_scope="${{ inputs.ut_name }}". The step is also still named xpu_distributed while it now handles two scopes.
• ut_result_check.sh:5 (header comment) and _linux_ut.yml:25 (the ut input description) still list the old scope set; the diff updated only the runtime error message.
• The PR description is stale: it describes xpu_distributed_bmg throughout, but the code uses xpu_distributed_weekly.

Recommendation

Request Changes

The artifact collision in pull.yml is demonstrated by this PR's own CI and makes the BMG result unreliable rather than additive, so it blocks on its own. Beyond that: the weekly scope runs unpinned, unreviewed code from a personal branch with secrets in the environment and mutates installed torch on a shared runner; the nightly substitution silently drops the existing distributed coverage; and linux-distributed is currently red with two unmuted failures.

Custom skills applied: pr-review.

---

Generated by review job.

[chuanqi129]

@zxd1997066 please address the review comment by bot

[chuanqi129]

@zxd1997066 please address the code conflict