test(ci): reclassify CPU-only sglang unit tests from gpu_1 to gpu_0 (OPS-8033) - #12718
Merged
Conversation
The sglang pre_merge amd64 GPU job's parallel stage brushed the 30-min step cap while its junit recorded zero seconds of test time. The stage's scheduler telemetry shows where the wall clock went: 353 items, 9,496 cumulative seconds, effective concurrency ~5.4, GPU idle at 0.5/22 GiB. 89% of that time is CPU-only unit tests marked gpu_1, each paying ~24s of per-item scheduling plus a fresh-interpreter sglang/torch import -- the same class of test runs in ~0.1s under the gpu_0 stage's loadscope-parallel pytest, which amortizes imports per worker. The gpu_1 marks were a proxy for "needs the sglang image", not for GPU use -- one file says so verbatim, another carries profiled_vram_gib(0) alongside gpu_1. The marker guidelines define gpu markers as the number of GPUs a test REQUIRES, and the proxy's fear is empirically obsolete: the arm64 CPU-only lane already runs `sglang and gpu_0` (205 passed in 96s) importing these same packages. Move the eight CPU-only files to gpu_0. The single CUDA-touching test (test_frames_are_pinned_on_cuda) keeps its skipif and stays consistent with gpu_0 = "no GPU required": it self-skips on the CPU lane and still guards pinning on the GPU job's gpu_0 stage. Serve deployments and router e2e keep gpu_1; they are the stage's real GPU work (667s + 212s cumulative) and now have the scheduler to themselves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
github-actions
Bot
added
test
backend::sglang
Contributor
WalkthroughThe SGLang test modules now use the ChangesSGLang test scheduling
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/src/dynamo/sglang/tests/test_sglang_nvdec_video_decoder.py`:
- Around line 31-36: Update the module-level pytest marker in
test_sglang_nvdec_video_decoder.py from gpu_0 to gpu_1 so
test_frames_are_pinned_on_cuda is included in the SGLang GPU job; leave the test
implementation unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 236e2054-91ab-45bb-b492-329f9fc55337
📒 Files selected for processing (8)
components/src/dynamo/frontend/tests/test_sglang_multimodal_prepost.pycomponents/src/dynamo/frontend/tests/test_sglang_processor_api.pycomponents/src/dynamo/frontend/tests/test_sglang_processor_unit.pycomponents/src/dynamo/frontend/tests/test_sglang_tool_calls.pycomponents/src/dynamo/sglang/tests/test_fpm_contract.pycomponents/src/dynamo/sglang/tests/test_sglang_multimodal_embedding_cache.pycomponents/src/dynamo/sglang/tests/test_sglang_nvdec_video_decoder.pycomponents/src/dynamo/sglang/tests/test_sglang_unit.py
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
…osts Verified on the amd64 image with the GPU masked: 14 of the file's tests fail without a GPU while passing with one, all through the same root -- parse_args reaches SGLang's ServerArgs.__post_init__, which auto-detects a device when --device is absent, and detection ends in NotImplementedError on a host with no accelerator (get_device -> SRTPlatform(unknown).get_device). The arm64 CPU lane never caught this because the file's old gpu_1 mark kept it off that lane entirely. An autouse fixture sets SGLANG_USE_CPU_ENGINE=1 (SGLang's public CPU knob, verified at v0.5.16: is_cpu() reads exactly this env var) only when CUDA is unavailable, clearing the lru_caches on both probes so worker-level cache state from other files cannot leak in either direction. GPU hosts keep exercising the real detection path unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
…sers Review finding on the gpu_0 reclassification: these files build a real Qwen tokenizer at module scope, and the move enrolled them in the amd64/arm64 CPU lanes where nothing warmed the HF cache -- every xdist worker would fetch it over the network, a flake surface this repo has been bitten by before. pytest.mark.model() is the declared mechanism: predownload_tokenizers downloads once per session under a file lock and the workers reuse the cache. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
rmccorm4
reviewed
Aug 5, 2026
rmccorm4
reviewed
Aug 5, 2026
rmccorm4
reviewed
Aug 5, 2026
rmccorm4
approved these changes
Aug 5, 2026
rmccorm4
left a comment
rmccorm4
left a comment
Contributor
There was a problem hiding this comment.
frontend test changes LGTM
rmccorm4
reviewed
Aug 5, 2026
…t.py Co-authored-by: Ryan McCormick <rmccormick@nvidia.com> Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
Co-authored-by: Ryan McCormick <rmccormick@nvidia.com> Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
…t.py Co-authored-by: Ryan McCormick <rmccormick@nvidia.com> Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
pytest.mark.model does not run any downloader itself. tests/conftest.py sweeps the marker into the session predownload manifest, and the tests/frontend gpu_0 modules that request predownload_tokenizers in the same CI session download that manifest and then flip HF_HUB_OFFLINE for the remainder of their worker. The marker is what keeps this tokenizer fetchable on such a worker. Say that, instead of claiming the marker activates the predownload. Raised by rmccorm4 on #12718. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
…nit-tests-gpu0 Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
nv-tusharma
approved these changes
Aug 6, 2026
dmitry-tokarev-nv
enabled auto-merge (squash)
August 6, 2026 17:59
nvda-mesharma
disabled auto-merge
August 6, 2026 20:45
This was referenced Aug 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
5 participants
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
The sglang pre_merge amd64 GPU job's parallel stage hit ~29.4 min (the 30-min step cap) while executing zero seconds of recorded test time. The wall clock went into 353 items — 9,496 cumulative seconds — trickling through the VRAM-aware scheduler at effective concurrency ~5.4 with the GPU idle (
GPU0: 0.5/22 GiB, 350+ queued). 89% of that cumulative time is CPU-only unit tests markedgpu_1, each paying ~24s of per-item scheduling + fresh-interpreter sglang/torch import.This moves the eight CPU-only files to
gpu_0, where the loadscope-parallel stage amortizes imports per worker (existing gpu_0 stage: 205 tests in 96s).Linear OPS-8033
Details
Observed on PR #12051's run 31036884612 (job 92413246330), from the stage's own telemetry and junit artifacts:
test_sglang_processor_unit.pytest_sglang_unit.pytest_sglang_tool_calls.pytest_sglang_multimodal_embedding_cache.pytest_sglang_processor_api.pytest_sglang_nvdec_video_decoder.pytest_sglang_multimodal_prepost.pytest_fpm_contract.pygpu_1marks were a proxy for "needs the sglang image" —test_sglang_unit.pysaid so verbatim, andtest_fpm_contract.pycarriedprofiled_vram_gib(0)alongsidegpu_1. Per.ai/pytest-guidelines.md, gpu markers mean GPUs required.pre_merge and sglang and gpu_0(205 passed / 96s in the same run) and imports these same packages (test_sglang_decode_handler,test_sglang_multimodal_video, …).test_frames_are_pinned_on_cuda) keeps itsskipif not torch.cuda.is_available()and stays consistent with gpu_0 = "no GPU required": it self-skips on the CPU lane and still guards pinning on the GPU job's gpu_0 stage.gpu_1— they are the stage's genuine GPU work (667s + 212s cumulative) and now get the scheduler to themselves. Expected parallel-stage wall drops from ~29 min to well under 10.Where should the reviewer start?
Any of the eight
pytestmarkblocks — the change is identical in each; the comment above the mark documents the reasoning in place.Validation
NVIDIA_VISIBLE_DEVICES=void), run from/workspacelike CI: all eight files — 351 passed / 1 skipped / 0 failed serially, identical result under-n auto --dist=loadscopein ~21s wall (the same tests cost ~8,400 cumulative seconds in the VRAM-scheduled stage). The one skip istest_frames_are_pinned_on_cudaskipping exactly as the gpu_0 contract requires.--collect-only -m "pre_merge and sglang and gpu_0"selects all 352.test_sglang_unit.pytests failed on an accelerator-less host becauseparse_argsreaches SGLang'sServerArgs.__post_init__, whose device auto-detection ends inNotImplementedErrorwhen no accelerator exists. The arm64 CPU lane never caught it because the oldgpu_1mark kept the file off that lane. An autouse fixture setsSGLANG_USE_CPU_ENGINE=1(SGLang's public CPU knob, verified at v0.5.16) only when CUDA is absent, withlru_cacheclears on both probes; GPU hosts exercise the real detection path unchanged.ruffclean; GPU-use audit per file: zero direct CUDA references except the self-skipping pinned test.Related Issues
Relates to the timeout observed on #12051.
🤖 Generated with Claude Code
Summary by CodeRabbit