Repository navigation
Activate the static consumer-graph parser for test selection - #8757
endurthiabhilash wants to merge 40 commits into
Conversation
Regenerate test_tools/therock_consumer_graph.json with generate_consumer_graph.py (the conservative may-depend superset: +4 nodes, +11 edges over the old cmake-emitted graph) and make the blocking `Test build_tools` pytest gate assert exact equality against the parser output. Retire the configure-based drift job (test_consumer_graph_drift.yml, check_consumer_graph_drift.py, and its test) — the parser test now enforces freshness without a full-tree configure. Sweep stale references to the removed workflow in comments.
generate_consumer_graph.py now also writes test_tools/therock_subtree_map.json (each external source subtree -> the graph key(s) built from it; 61 entries), and --check diffs both committed files, exiting non-zero on any drift. Add a blocking real-tree test asserting the committed subtree_map equals the parser output. The node graph file stays flat and byte-identical, so the graph loaders (which treat every top-level key as a node) are unaffected. The artifact/source-dir map lands in 6b alongside its consumer.
determine_rocm_test_dependencies.py now reads test_tools/therock_subtree_map.json and layers a 14-entry _SUBTREE_ALIAS_OVERRIDES on top, replacing the 25-entry hand _EXTERNAL_SUBTREE_ALIASES. 11 derivable entries are retired; the residue the parser cannot derive (non-declared subprojects, the nested hipblaslt/tensilelite path, name skews like cuid->rdc, and the variable-sourced amd-dbgapi) stays as overrides. Superset-safe: a parity test freezes the full old dict and asserts no under-selection for any of its 25 keys. Exactly one selection change — projects/clr now selects [hip-clr, ocl-clr] (was [hip-clr]; clr builds both, so the old value under-selected). _CI_TEST_SELECTOR_ALIASES is left for commit 7.
Parses therock_provide_artifact; --check covers it. No selection change.
…tion Adds a drift test; documents kfdtest/composable-kernel. No behavior change.
Parser is authoritative; drop therock_emit_consumer_graph and its CMakeLists call.
Report-only source-dir map parsing no longer aborts the authoritative graph.
Lazy friendly-error load; guard overrides against shadowing generated edges.
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Pre-commit check failed⛔ pre-commit failed Please run locally:
This repo uses |
|
🎉 All checks passed! This PR is ready for review. |
Drop profiler-hub, drop rocjitsu edge, add rocwmma.
Five test-matrix jobs (hipdnn_install, hipthreads_examples, miopen-dbsync, libhipcxx_amdclang, libhipcxx_hiprtc) have no graph key and no selector alias, so they only run when selection is "*". Scoped PR runs skip them, and a libhipcxx change selects no jobs at all. Alias each onto the component it exercises (plus hipdnn-cudnn-samples, which exists on main), and add a guard that fails when any test-matrix job is unreachable from every component. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Hi Abhilash! I put up #8990 against this branch. A handful of test-matrix jobs (hipdnn_install, hipthreads_examples, miopen-dbsync, and both libhipcxx jobs) aren't reachable from any component, so they only run on full I'm not really thinking of it as a stack. I think it fits naturally as part of this PR, since you're already reworking the alias layer. Would you be up for merging #8990 into your branch (or cherry-picking the one commit, whichever you prefer)? One heads-up: on this branch as it stands, the guard flags |
Regenerate test_tools/therock_consumer_graph.json with generate_consumer_graph.py (the conservative may-depend superset: +4 nodes, +11 edges over the old cmake-emitted graph) and make the blocking `Test build_tools` pytest gate assert exact equality against the parser output. Retire the configure-based drift job (test_consumer_graph_drift.yml, check_consumer_graph_drift.py, and its test) — the parser test now enforces freshness without a full-tree configure. Sweep stale references to the removed workflow in comments.
generate_consumer_graph.py now also writes test_tools/therock_subtree_map.json (each external source subtree -> the graph key(s) built from it; 61 entries), and --check diffs both committed files, exiting non-zero on any drift. Add a blocking real-tree test asserting the committed subtree_map equals the parser output. The node graph file stays flat and byte-identical, so the graph loaders (which treat every top-level key as a node) are unaffected. The artifact/source-dir map lands in 6b alongside its consumer.
determine_rocm_test_dependencies.py now reads test_tools/therock_subtree_map.json and layers a 14-entry _SUBTREE_ALIAS_OVERRIDES on top, replacing the 25-entry hand _EXTERNAL_SUBTREE_ALIASES. 11 derivable entries are retired; the residue the parser cannot derive (non-declared subprojects, the nested hipblaslt/tensilelite path, name skews like cuid->rdc, and the variable-sourced amd-dbgapi) stays as overrides. Superset-safe: a parity test freezes the full old dict and asserts no under-selection for any of its 25 keys. Exactly one selection change — projects/clr now selects [hip-clr, ocl-clr] (was [hip-clr]; clr builds both, so the old value under-selected). _CI_TEST_SELECTOR_ALIASES is left for commit 7.
Parses therock_provide_artifact; --check covers it. No selection change.
…tion Adds a drift test; documents kfdtest/composable-kernel. No behavior change.
Parser is authoritative; drop therock_emit_consumer_graph and its CMakeLists call.
Report-only source-dir map parsing no longer aborts the authoritative graph.
Lazy friendly-error load; guard overrides against shadowing generated edges.
Drop profiler-hub, drop rocjitsu edge, add rocwmma.
13ca00c to
bf34239
Compare
|
Thanks for the PR Tony. I don't have any issues merging your PR into this branch. Once I merge it, I still have to do a few more tests to make this PR ready for review. |
JIRA ID: ALMIOPEN-3021 ## Motivation `miopen-dbsync` (GPU-free rocjitsu StaticFDBSync, #8813 added gfx950 as advisory) is never scheduled for PR-style runs. A rocm-libraries PR touching `projects/miopen` selects `projects_to_test=hipdnn-integration-tests,hipdnn-samples,miopen,miopenprovider` at `test_type=standard`; `fetch_test_configurations.py` only schedules a job whose key is in that list (or `*`), so `miopen-dbsync` only runs in nightlies (`*`) or manual dispatch. Verified on a throwaway rocm-libraries PR against TheRock `b0958085`: the gfx94X "Configure test matrix" log requested sanity, miopen, hipdnn-integration-tests, hipdnn-samples, miopenprovider and no `miopen-dbsync`. ## Technical Details Add `"miopen": ["miopen", "miopen-dbsync"]` to `_CI_TEST_SELECTOR_ALIASES` in `test_tools/determine_rocm_test_dependencies.py` (same pattern as `tensilelite` -> `tensilelite-common`). `miopen` stays selected; `miopen-dbsync` is added whenever `miopen` is in the result. The job still self-limits to standard/comprehensive/full tiers, gfx942 (blocking) and gfx950 (xfail) on the CPU runner, so quick-tier and other families are unchanged. This is the same alias #8990 adds, but #8990 merged into the stacked branch of #8757 and so is not on `main`; this PR does not depend on #8757. If #8757 lands later it should carry the same alias (its reachability guard already requires it). ## Test Plan - Unit test `test_cli_selects_miopen_dbsync_with_miopen`: `--changed-projects miopen` outputs exactly `{miopen, miopen-dbsync}`. - Run the real selection scripts locally for gfx94X-dcgpu and gfx950-dcgpu at standard and quick tiers. - End to end in rocm-libraries CI: a PR pinned to a TheRock commit containing this change, with a MIOpen source change. ## Test Result - `cd test_tools && python3 -m unittest tests.determine_rocm_test_dependencies_test`: 75 tests OK (74 before). - `build_tools/github_actions/tests/fetch_test_configurations_test.py`: passes unchanged. - Local scripts: `determine_rocm_test_dependencies.py --changed-projects projects/miopen --gha-output` now emits `hipdnn-integration-tests,hipdnn-samples,miopen,miopen-dbsync,miopenprovider`. `fetch_test_configurations.py --platform=linux` at `TEST_TYPE=standard` requests `miopen-dbsync` on the CPU runner for both gfx94X-dcgpu and gfx950-dcgpu; at `quick` it logs `Excluding job miopen-dbsync` for both. - End to end: throwaway rocm-libraries PR #14288 (closed) pinned TheRock to `2104ff3c` and touched a `projects/miopen` source file. `2104ff3c` was this change plus a prototype that treats `.github/actions/ci-env/*` as a workflow change in `configure_external_repo_ci.py`. That prototype is NOT part of this PR. It was needed only because a pin bump edits `ci-env/action.yml`, which otherwise forces run-all at quick tier, where `miopen-dbsync` is skipped by design. Run: https://github.com/ROCm/rocm-libraries/actions/runs/39985392063 - `Test miopen-dbsync (gfx94X-dcgpu)`: passed. StaticFDBSync gfx942 @ 304 CU and @ 228 CU, each `[ PASSED ] 1 test`. - `Test miopen-dbsync (gfx950-dcgpu) (xfail)`: passed. StaticFDBSync gfx950 @ 256 CU, `[ PASSED ] 1 test`. - Both ran on the CPU runner `aws-linux-scale-rocm-prod`; the gfx94X "Configure test matrix" log showed `TEST_TYPE: standard` and `Requesting job miopen-dbsync`. - Other jobs in that run were not waited on; the run was cancelled after the dbsync jobs finished. - Not run: `label_consistency_test` (module did not import in my environment, before or after this change), pre-commit. ## Risk Assessment **Risk Level: Low.** CI selection only; adds one job to MIOpen-touching PR runs, CPU runner, gfx950 non-blocking.
|
Hi Abhilash! I put up #9539 against your branch. It's a small change to the subtree map: The reason is ROCm/rocm-libraries#14048. @ScottTodd's feedback there was that TheRock should know how to scope a scheduled external-repo run to that repo's own code, not each repo parsing its own I figured it's easier to change the file format before it lands than after, which is why I aimed it at your branch. Most of the diff is the regenerated JSON and test updates. If you'd rather not grow this PR, no worries at all. I can rebase it onto main as a follow-up once #8757 merges. One small thing I noticed while tagging the overrides: |
|
Thank you @tony-davis, I'll check and merge the changes to my branch. |
geomin12
left a comment
geomin12
left a comment
There was a problem hiding this comment.
few comments, overall looks pretty good, however, reviews + merges will go a lot faster if this is split up. perhaps we add the trees + logic, then make the transition in a follow-up? (deletion).
There was a problem hiding this comment.
for subtrees, should we just map the ones that are mismatched? such as:
"dnn-providers/integration-tests": [
"hipdnn_integration_tests"
],
There was a problem hiding this comment.
oh i thought cmake parser would get this too. is this needed?
Take main's hipccl subtree overrides and selector aliases, regenerate the consumer-graph maps, delete test_consumer_graph_drift.yml (main only bumped runners/actions in it), add HARDENING_DEFERRED to the parser keyword set, and let the reachability guard honor base_project (hipfft/rocfft multi-gpu).
…umer Remove therock_source_dir_map.json, its generation and --check coverage, the therock_provide_artifact parsing, the related tests, and the RFC0013 text. It returns with the BuildTopology change that reads it.
Motivation
Follow-up to #8199. That PR added the parser report-only; this one makes the generated maps drive test selection and removes the configure-time CMake emit.
GitHub issue: #7782
Technical Details
generate_consumer_graph.py: generates all three committed maps (therock_consumer_graph.json,therock_subtree_map.json,therock_source_dir_map.json)determine_rocm_test_dependencies.py: resolves subtrees from the committedsubtree_mapplus a hand-override layer, replacing the static alias dict. The map is loaded once from--therock-dir(falling back to the committed copy), so subtree resolution follows the same source as the graph.hipdnn_install,hipthreads_examples,libhipcxx_amdclang,libhipcxx_hiprtc,miopen-dbsync, plushipdnn-cudnn-samples) are aliased onto the components they exercise, and a guard fails when any test-matrix job is unreachable from every component. Previously these ran only under*.cmake_consumer_graph.py: unresolvableSUBPROJECT_DEPSare omitted with a diagnostic, not fatal.therock_emit_consumer_graph.cmakeand the configure-time emit, plustest_consumer_graph_drift.ymlandcheck_consumer_graph_drift.py.Test Plan
cd build_tools && python3 -m pytest tests/ github_actions/tests/cd test_tools && python3 -m pytestpython3 build_tools/generate_consumer_graph.py --checkTest Result
--checkexits 0.Submission Checklist