[CI] Parity: fold CUDA distributed-test split (pytorch#189232) into distributed config#3432
Open
ethanwee1 wants to merge 12 commits into
Open
[CI] Parity: fold CUDA distributed-test split (pytorch#189232) into distributed config#3432ethanwee1 wants to merge 12 commits into
ethanwee1 wants to merge 12 commits into
Conversation
…main download_testlogs required exactly one of --pr_id or --sha1 and rejected the "neither" case, so dispatching parity with an empty/"latest" sha (the documented "latest green on main" option, e.g. a baseline_sha comparison) failed with "Please provide either pr_id or sha!". Treat an empty --sha1 or the literal "latest" as "resolve latest green run on main": look up the newest successful ROCm default-workflow run on main (download_workflow_run already does this when given no head_sha) and use its head commit as the target sha. The parity.yml side already omits --sha1 for "latest", so no workflow change is needed. Also clarifies the both-provided error message.
Auto-parity was trunk-scoped (mi350 only). mi300/mi200/navi31 run in their own scheduled upstream workflows on their own SHAs at a different cadence, so the trunk-push scan never reached them. - Discover candidate SHAs from both trunk.yml pushes and the scheduled per-arch workflows (fetch_scheduled_commits / fetch_candidate_commits), deduped newest-first; a 40-hex guard drops stray non-row output. - Hold back SHAs newer than the newest scheduled run so a lagging mi300/mi200/navi batch can still join that SHA's report, yielding ONE combined parity report per SHA (parity.yml's matrix already emits a per-arch artifact plus a merged summary) once every arch that ran has finished. When no scheduled runs exist, nothing is held (mi350/trunk behaves exactly as before). - Expand ARCHS_IN to mi350 mi300 mi200 navi31. - Per-config CUDA-baseline gating: a SHA whose CUDA jobs for a test config did not run (e.g. a failed trunk run that never launched CUDA default) has no baseline for that config, so exclude just that config from the dispatch (via parity.yml's exclude_* inputs) instead of emitting a bogus all-MISSED column, and drop its ROCm shards from the completion gate. No parity_job_config.json change: mi300 has no fallback; its default/ distributed/inductor come from rocm-mi300 / periodic-rocm-mi300 / inductor-rocm-mi300, which run together on one scheduled SHA.
Shard counts were hardcoded (config value or literals), so when an arch falls back to a different workflow whose sharding differs, the constructed "(config, i, N)" job/artifact keys miss the real jobs. Concretely mi200's default/inductor workflows are dormant upstream and fall back to trunk-rocm-sandbox, which shards default into 10 (config says 6) and inductor into 4 (config says 2) - so every key "DOES NOT EXIST IN JOBS" and the mi200 shard of a combined auto report fails. Add derive_shard_count(), which reads the actual shard total from the resolved run's job names, and use it for ROCm default/distributed/inductor (falling back to the config value only when no matching jobs are found). This unblocks mi200 in the multi-arch auto-trigger.
The DISAGREE/AGREE metric counted a ROCm SKIPPED or MISSED test as a
disagreement whenever CUDA merely did not SKIP it - which includes the
large set of attention-backend parametrization variants CUDA never even
enumerates (CUDA MISSED). Those are not real ROCm-vs-CUDA coverage gaps and
inflated DISAGREE to ~3% (AGREE ~97%).
Count a disagreement only when CUDA actually PASSED the test (s2 == PASSED)
in both compute_test_config_stats and compute_overall_stats, and relabel the
two line items accordingly ('PASSED on <set2>'). This matches the triage
spreadsheet's adjusted definition; AGREE% becomes ~99%.
The HUD link added in pytorch#3258 was lost when parity.yml was rewritten. Re-add it in generate_summary.py (next to the Commit SHA header) so it lives in the report itself (step summary + CSV artifact) and survives workflow rewrites. The link targets the upstream HUD page for the resolved commit, regex-filtered to the trunk CUDA/inductor/rocm test jobs the report is built from. Parens/pipes are percent-encoded to keep the markdown link valid; normal runs already pass set1_name=rocm/set2_name=cuda. Verified end-to-end against a real mi350 status CSV.
Consumer-side support for commit-vs-commit parity (baseline_sha mode): - detect_log_failures.py: preserve the short commit-SHA prefix on log filenames (e.g. 09e0c59b_rocm3.txt) as the platform label, matching the SHA-prefixed filenames download_testlogs produces in baseline mode. - summarize_xml_testreports.py: name the per-test-file running-time CSV columns from the resolved set names (set1/set2) instead of hardcoded rocm_/cuda_, and add per-config test-shard counts. The download_testlogs run-selection side of this work already landed in develop via pytorch#3278, so this PR carries only the remaining consumer-side parsing changes.
The running-time CSV columns use set1_name/set2_name (rocm/cuda by default, commit SHAs in commit-vs-commit mode), but the printed summary still hardcoded ROCM/CUDA. Drive the user-facing summary headers from the same set names so both stay consistent; normal-mode output is unchanged (rocm/cuda).
Adds a "preview" ROCm arch so parity can be run against the upstream rocm-preview lane (linux-noble-rocm-preview-py3.12-gfx942, gfx942), which runs the TheRock preview wheel (e.g. ROCm 7.14) via the ciflow/rocm-preview label. default/distributed/inductor all come from the single rocm-preview workflow (6/3/2 shards). - parity_job_config.json: new rocm.preview entry. - download_testlogs: allow --arch preview. - parity.yml: list preview as an arch option. Because rocm-preview is ciflow-triggered (not scheduled on main), this arch is for manual/on-demand parity runs against a rocm-preview commit; it is intentionally not added to the auto-trigger scope.
test_conv2d_backward_parametrized is skipped on ROCm via skipIfRocmArch (upstream pytorch#188671, CI timeout). It was falling into the generic Misc bucket; add a Tier-1 rule so conv2d_backward gfx-arch skips are categorized as 'PT2.0 - Convolution' instead. Non-conv2d gfx skips stay Misc.
Flip the parity.yml auto_classify input default false->true so skip-reason classification runs on every dispatch, including autoparity commits (which don't pass the input and so inherit the default). Previously auto runs left skip_reason blank; now the disagreement subset (SKIPPED/MISSED on ROCm + PASSED on CUDA) is categorized automatically.
…istributed config pytorch#189232 hived single-GPU distributed tests out of the 'distributed' config onto CUDA's 1-GPU 'default' runner, while ROCm still runs the whole distributed suite under 'distributed'. Our (test, config) keying then can't match a CUDA-'default' result with the ROCm-'distributed' result, so the same passing test double-counts as MISSED on both sides (~1700 spurious mi350 disagreements starting 07/11). Canonicalize default -> distributed for distributed.* test files so the two stacks line up again. Scoped to distributed.* files only, leaving genuine default/inductor results untouched.
|
Jenkins build for 502b08a27495d7edc7e11f542431ce4d5f5c229e commit finished as FAILURE |
The distributed-config fold (pytorch#189232) was applied to the per-test counts but not to the per-file running-time aggregation, which keys config from the testsuite dict (index 2) rather than the folded testcase dict. That left distributed.* files with a phantom 'default' row carrying the CUDA default-shard import time but 0 tests run (e.g. test_fully_shard_comm: default cuda_running_time=48.9, cuda_tests_run=0). Factor the mapping into _canonical_distributed_config() and apply it in the ROCm/CUDA running-time and shard aggregation loops so per-file times fold into 'distributed' alongside the counts. No phantom 0-test rows; times and counts are now consistent. Disagreement counts are unchanged.
|
Jenkins build for 261423947b8fcaf829ad6b14dee241a9f120c207 commit finished as FAILURE |
2 tasks
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
Distributed parity disagreements on all ROCm arches jumped ~1700 starting 07/11 (e.g. mi350 went 3614 → 4796). The cause is not a regression — it's a config-attribution artifact from upstream pytorch#189232 ("Add multigpu pytest marker to partition distributed tests by GPU count").
That PR hived the single-GPU distributed tests out of the
distributedconfig and onto CUDA's cheaper 1-GPUdefaultrunner, while ROCm (and CPU) still run the whole distributed suite underdistributed. Because parity keys on(test_file, test_class, test_name, test_config), a CUDAdefaultresult can no longer line up with the matching ROCmdistributedresult, so the same passing test double-counts as MISSED on both sides.Fix
_fold_distributed_config()canonicalizesdefault→distributedfordistributed.*test files (applied to both the ROCm and CUDA parsed test-case dicts). Scoped todistributed.*files only, so genuinedefault/inductorresults are untouched (no risk of masking a real per-config disagreement).Validation
End-to-end parity run on the same commit (
cac2394a, mi350), with vs without the fix:All removed rows are single-GPU
distributed.*tests that pass on both stacks; nodefault/inductorcategories moved.Test plan
cac2394ami350 confirms 4796 → 3061default↔distributed;inductorunaffectedpython -m py_compilepassesDeployment status
Merged to the fork
ethanwee1/pytorch:main(0c9facbfa2d) so it is live on autoparity while this PR is under review. This PR remains the upstream landing target forROCm/pytorch:develop.Follow-up: running-time report consistency
The fold was initially applied only to per-test counts, which left
distributed.*files with a phantomdefaultrow in the running-time report (CUDA default-shard import time but 0 tests run). A follow-up commit factors the mapping into_canonical_distributed_config()and applies it to the running-time/shard aggregation too, so per-file times fold intodistributedalongside the counts. Verified end-to-end: phantom rows 253 → 0, disagreements unchanged at 3061.Fork
maindeployment SHA updated toedb53c99992.