Repository navigation
[AMD][CI] Restore AMD registrations dropped by #37436 and fix the 3FS hicache suite - #40060
Closed
michaelzhang-ai wants to merge 4 commits into
Closed
michaelzhang-ai wants to merge 4 commits into
michaelzhang-ai wants to merge 4 commits into
Conversation
#37436 collapsed each of these files' two registrations into one and kept only the CUDA line, folding the AMD `est_time` into it (e.g. CUDA 38 -> 77 in test_engine_child_pids.py). The AMD coverage was not retired on purpose; #38581 restored five such files and these seven are the remainder. Each registration is restored to the suite and `est_time` it carried before #37436. All seven target `stage-b-test-1-gpu-small-amd`, which pr-test-amd.yml dispatches across 14 partitions, so the added 489s of est_time works out to roughly 35s per partition. Originally registered by: - test_engine_child_pids.py #25208 (AMD) - test_http2_server.py #24572 (AMD) - test_request_queue_validation.py #16426 - test_priority_metrics.py #21740 - test_large_max_new_tokens.py #16326 - test_matched_stop.py #16326 - test_request_length_validation.py #16326 Co-authored-by: Michael <michaelzhang-ai@users.noreply.github.com>
The AMD registration has named a non-AMD suite since it was added in #17116, where the sibling file/backend test got `stage-b-test-large-2-gpu-amd` but this one was written without the `-amd` suffix. #25420's mechanical `stage-` -> `base-` rename then carried the AMD line along with the CUDA one, leaving `suite="base-b-test-2-gpu-large"` -- a CUDA suite name that no AMD workflow invokes, so the test has never run on AMD. Retarget it at `stage-b-test-2-gpu-large-amd`, matching test_hicache_storage_file_backend.py, whose mixin this test derives from. The 3FS client is mocked (`use_mock_hf3fs_client`), so no 3FS hardware is required. Co-authored-by: Michael <michaelzhang-ai@users.noreply.github.com>
`run_suite.py --hw amd` selects tests on effective_suite, and AMD sits outside run_suite.py's `_SUITE_CHECKED_BACKENDS`, so an AMD registry pointing at another backend's suite name matches nothing and the test silently reports nothing rather than failing. That is how the 3FS hicache test went eight months without running on AMD. Extend the registered-test hook with the AMD analogue of the check it already applies to non-dispatchable CUDA suites: an AMD suite name must carry `amd` as a hyphen-delimited token, or be a `nightly-*` suite (nightly-test-amd.yml names those after the model, not the vendor). All 124 AMD suite names currently in test/registered/ satisfy this; the only name that did not was the 3FS typo fixed in the previous commit. Co-authored-by: Michael <michaelzhang-ai@users.noreply.github.com>
Resolve a docstring-only conflict in scripts/lint/check_registered_tests.py. main shortened the module docstring; keep that form. The AMD-suite lint check from this branch was already auto-merged. Co-authored-by: quitenode <quitenode@users.noreply.github.com>
This was referenced Sep 21, 2026
This was referenced Sep 29, 2026
This was referenced Oct 7, 2026
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
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.
Motivation
Eight test files carry an
register_amd_cicall that never dispatches, so the AMD coverage their owners registered does not run. Two independent causes:1. #37436 dropped AMD registrations. That PR collapsed each file's two registrations into one, kept only the CUDA line, and folded the AMD
est_timeinto it (e.g.test_engine_child_pids.pyCUDAest_timewent 38 → 77, the old AMD value). The AMD coverage was not retired on purpose. #38581 restored five such files; these seven are the remainder:core/test_engine_child_pids.pyopenai_server/basic/test_http2_server.pycore/test_request_queue_validation.pyobservability/test_priority_metrics.pyopenai_server/validation/test_large_max_new_tokens.pyopenai_server/validation/test_matched_stop.pyopenai_server/validation/test_request_length_validation.py2. The 3FS hicache test has never run on AMD. Its AMD registration named a non-AMD suite from the day it was added in #17116 — the sibling file-backend test got
stage-b-test-large-2-gpu-amd, this one was written without the-amdsuffix. #25420's mechanicalstage-→base-rename then carried the AMD line along with the CUDA one, leavingsuite="base-b-test-2-gpu-large": a CUDA suite name that no AMD workflow invokes.Nothing caught either regression.
run_suite.pyselects oneffective_suite, and AMD sits outside its_SUITE_CHECKED_BACKENDS, so a suite name belonging to another backend matches zero tests and the job printsNo tests found ... Skippinginstead of failing.Modifications
Three commits, one per logical change:
Restore the seven dropped registrations, each at the suite and
est_timeit carried before [Test] Consolidate test cleanup and CI taxonomy (net -11.4K lines) #37436. All seven targetstage-b-test-1-gpu-small-amd. CUDA/CPU registrations are untouched — theest_timevalues [Test] Consolidate test cleanup and CI taxonomy (net -11.4K lines) #37436 folded into the CUDA lines are refreshed weekly fromsglang-ci-stats, so they self-correct.Retarget the 3FS hicache AMD registration at
stage-b-test-2-gpu-large-amd, matchingtest_hicache_storage_file_backend.py, whose mixin this test derives from. The 3FS client is mocked (use_mock_hf3fs_client), so no 3FS hardware is required.Add the AMD analogue of the existing non-dispatchable-CUDA-suite check to
scripts/lint/check_registered_tests.py: an AMD suite name must carryamdas a hyphen-delimited token, or be anightly-*suite (nightly-test-amd.ymlnames those after the model rather than the vendor, e.g.nightly-perf-8-gpu-grok2). All 124 AMD suite names currently intest/registered/satisfy this rule; the only violation was the 3FS typo, so the guard is a no-op onmainapart from catching recurrences.Accuracy Tests
No model, kernel, or forward-path code is touched; this is CI registration metadata plus a lint rule.
Speed Tests and Profiling
Both affected jobs keep comfortable headroom:
timeout-minutesstage-b-test-1-gpu-small-amdstage-b-test-2-gpu-large-amdVerified locally that strict collection still succeeds over all 2491 registrations, that each restored registration resolves to
stage-b-test-1-gpu-small-amdwith an executable__main__entry, and that the new lint rule fails on the pre-fix 3FS suite name and passes on the whole tree after it.Because these tests have not run on AMD hardware recently (and the 3FS one never has), please trigger the AMD suites on this PR so the restored files are validated before merge.
Checklist
Notes for reviewers
Per
write-sglang-test,register_amd_cishould not be added unless a test exercises ROCm paths. This PR adds no new coverage decisions — every registration here restores one an AMD CI owner already made, at its original suite andest_time. If any of the seven is no longer wanted on AMD, the right outcome is an explicitdisabled="reason"rather than the current silent drop.Two adjacent gaps found while auditing, left out of scope:
nightly-amd-8-gpuandnightly-amd-vlmare inNIGHTLY_SUITESwith zero registered tests and no workflow dispatch, and 94 of the 124 registered AMD suite names are absent fromrun_suite.py's catalog (harmless today only because AMD is exempt fromvalidate_all_suites). Adding AMD to_SUITE_CHECKED_BACKENDSwould make the catalog authoritative, but it requires cataloguing all 94 names and adds friction for AMD contributors, so it belongs in a separate PR with AMD CI owners' input.CI States
Latest PR Test (Base): ❌ Run #35623020492
Latest PR Test (Extra): ❌ Run #35623020053
Latest PR Test (AMD ROCm 10): ❌ Run #35623020472