[CI] Parity: fix mi350 inductor reading rerun_disabled trunk run (50k phantom MISSED)#3447
Open
ethanwee1 wants to merge 17 commits into
Open
[CI] Parity: fix mi350 inductor reading rerun_disabled trunk run (50k phantom MISSED)#3447ethanwee1 wants to merge 17 commits into
ethanwee1 wants to merge 17 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.
The log-based flaky detector inferred flakiness from the last individual "... PASSED" line seen before a "Test succeeded in new process" marker. That heuristic attaches the flaky flag to whatever test happened to pass most recently, which is often an unrelated test that passed normally much earlier in the shard -- not the test that actually crashed/failed and then passed on rerun. Concrete example (run 29255582284, mi350, inductor/test_compiled_autograd): test_wrap_recreates_contexts_for_repeated_runs passed normally at ~16% and was wrongly reported as flaky, while the test that truly failed-then-passed on rerun was FuncTorchHigherOrderOpTestsWithCompiledAutograd::test_jacfwd. run_test.py already prints an authoritative per-shard summary naming the exact tests: The following tests failed and then succeeded when run in a new process ['test/inductor/test_compiled_autograd.py::...::test_jacfwd'] Parse that line directly instead of guessing. Verified against the real ROCm and CUDA logs from the run above: test_wrap is no longer flagged and test_jacfwd is correctly reported on both platforms.
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.
… job failed When an upstream config's jobs are missing (e.g. ROCm distributed test job skipped during GPU capacity crunches), download_testlogs exits non-zero and, with `set -o pipefail`, aborted the whole generate-parity job before the CSV was built or uploaded -- so no report at all for that arch. Capture download_testlogs' real exit code instead of aborting, let the report generate and upload from whatever downloaded (default/inductor), then fail the job at the end via a dedicated "Flag incomplete download" step. Net effect for e.g. mi300 with distributed off: the per-arch report + artifact are still produced (and summarize still aggregates via `if: !cancelled()`), but the arch job is clearly marked failed so the incomplete download is visible.
… the run mi300's default/distributed/inductor come from three separately-scheduled workflows (rocm-mi300 / periodic-rocm-mi300 / inductor-rocm-mi300) that do not always land on the same SHA. When one config's workflow run did not run for a SHA, download_testlogs raised "workflow not found ... increasing max_pages" and aborted the whole download -- dropping the configs that DID run and producing no report. (The message was also misleading: download_workflow_run already filters by head_sha, so it's not a pagination problem -- the run genuinely doesn't exist for that SHA.) The parity-auto gate can't prevent this per-arch: it dispatches arches in one combined run, so its dispatch-level exclude flags can't drop a config for just one arch. Handle it in download_testlogs (which runs per-arch): when a config's ROCm workflow (default/distributed/inductor) or the CUDA inductor baseline is absent for the SHA, log it, record it in error_msgs, and skip just that config. The report is still built from the available configs, and the job is still flagged failed at exit so the gap stays visible.
Each trunk ROCm shard has, besides the normal test job, "variant" jobs that share the same "(config, shard, total" name and the same test-reports-<config>-<shard>-<total> artifact prefix: rerun_disabled_tests (re-runs only the currently disabled/flaky tests) and mem_leak_check. The job matcher grabbed the first name match, which could be the variant, and the artifact filter only excluded names literally containing "rerun_disabled" -- but the variant's S3 artifact is named test-reports-test-inductor-1-2-<runner>_<jobid> with no marker, so it slipped through. Net effect on mi350 inductor: parity used the rerun_disabled job's ~10 flaky reruns instead of the normal job's full suite, so ~50,840 of CUDA's inductor tests showed as MISSED. Track variant job IDs in get_workflow_jobs and exclude the variants from job matching (write_test_log_to_file), shard-count derivation, and the S3 artifact filter (by name AND by "_<variant jobid>"). Parity now uses the normal job's full results.
…dic rerun_disabled trunk run) The ROCm inductor block resolved its own trunk run via download_workflow_run, which could return the periodic rerun_disabled_tests/mem_leak_check trunk run (only variant inductor jobs) instead of the canonical push run. Reuse trunk_full_wf like default/distributed already do.
|
Jenkins build for 579ba57cd66190453702eed0fd7339d25160e2a1 commit finished as FAILURE |
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
mi350 inductor showed ~50,840 tests as
MISSEDagainst the CUDA baseline. It wasn't a coverage gap — parity was reading the wrong ROCm inductor jobs.Two problems, both fixed here:
Wrong trunk run for ROCm inductor. The inductor block resolved its own trunk run via
download_workflow_run(workflow="trunk", …), which could return the periodicrerun_disabled_tests/mem_leak_checktrunk run — that run only has the variant inductor jobs (a handful of re-run flaky tests), not the normal full suite. Default/distributed already reuse the canonical push run (trunk_full_wf); inductor now does too.Variant jobs matched/kept generally. Variant jobs share the normal shard's
(config, shard, totalname and thetest-reports-<config>-<shard>-<total>artifact prefix, and their S3 artifact name has norerun_disabledmarker (just_<jobid>), so the old name-only filter missed them. Now variant job IDs are tracked inget_workflow_jobsand excluded from job matching, shard-count derivation, and the artifact filter (by name and by_<variant jobid>).Validation (mi350,
f877a09)MISSEDgenerate-parity (mi350)now succeeds; inductor lines up with mi300 (~267). Log confirms ROCm inductor now reads from the canonical push run.Test plan
python -m py_compilef877a09: inductor 50,840 MISSED → 0inductor-rocm-*workflow (mi300)