Bump sglang to v0.5.16 - #1795
Open
yueming-yuan wants to merge 26 commits into
Open
Conversation
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
yueming-yuan
added
bypass-fastfail
pr-test.yml does not listen for the 'edited' event, so the run started by the label additions captured the PR body before 'ci-sglang-pr: sglang-miles-v0.5.16' was added and would have resolved sglang to the default 'sglang-miles' branch.
sglang v0.5.16 added `function_call/inkling_detector.py`, which imports `from xgrammar import StructuralTag` at module level, and `function_call_parser.py` imports that detector unconditionally. miles' conftest reaches the parser through `mock_sglang_server`, so every CPU stage failed at collection with `ModuleNotFoundError: No module named 'xgrammar'`. The hosted CPU runner puts sglang on PYTHONPATH from source without installing sglang's own dependencies, which is exactly what this file exists to mirror. xgrammar is a hard sglang dependency (`xgrammar==0.2.1` in its pyproject, not an extra); pinned to the same version. GPU/ROCm stages already get it from the sglang base image. Note `function_call/base_format_detector.py` guards the same import with try/except ImportError, so the rest of that module family tolerates a missing xgrammar; the new inkling detector does not. Checked the whole delta rather than fixing one symbol at a time: walking every unguarded module-level third-party import reachable from the CPU suite's sglang entry points, xgrammar is the only one v0.5.16 adds over v0.5.15.
…ving double
`_make_serving` builds an OpenAIServingChat via `object.__new__` and sets only the
attributes `_process_messages` reads, so every attribute `__init__` gains has to be
mirrored here. v0.5.16 added server-level `default_chat_template_kwargs`, merged into
the request's `chat_template_kwargs` at the top of `_process_messages`, which made all
174 TestAlignWithSGLang / TestDatasetRouting cases fail with
AttributeError: 'OpenAIServingChat' object has no attribute
'default_chat_template_kwargs'
Mirrored as `{}` (what `__init__` produces when the server arg is unset) so the
request's own kwargs stay the only ones applied. Same pattern as the existing
`_tokenizer_auto_adds_specials` line added for v0.5.13.
Diffed every `self.X =` in OpenAIServingChat.__init__ between v0.5.15 and v0.5.16:
`default_chat_template_kwargs` is the only addition, and `_make_serving` is the only
hand-built construction site in the repo.
3 of the 4 failing GPU stages died during CUDA-graph capture with
File nvidia_cutlass_dsl/.../cutlass_dsl/tvm_ffi_provider.py, line 659
self._kwargs_wrapper = kwargs_wrapper.make_kwargs_wrapper(
TypeError: make_kwargs_wrapper() got an unexpected keyword argument
'map_dataclass_to_tuple'
reached from layernorm.forward_cuda -> sgl_kernel rmsnorm -> flashinfer's CuTe
rmsnorm kernel. Not a migration regression: these jobs are green on main, and the
trigger is a dependency bump inside sglang v0.5.16 itself --
nvidia-cutlass-dsl 4.5.2 -> 4.6.0
flashinfer_python 0.6.12 -> 0.6.14
apache-tvm-ffi 0.1.11 -> 0.1.11 (unchanged)
`map_dataclass_to_tuple` was added to apache-tvm-ffi in 0.1.10 (verified by
diffing the 0.1.9/0.1.10/0.1.11/0.1.12 sdists), so the TypeError means the
apache-tvm-ffi actually resolved at runtime is <= 0.1.9 -- older than the 0.1.11
sglang pins. cutlass-dsl 4.5.2 did not pass that kwarg, which is why v0.5.15 was
unaffected. Something among this file's many later pip installs drags in the
older build.
Restores sglang's own pin after every other install. The step verifies the
signature it depends on and fails the build if the pin does not stick, so a
future downgrade surfaces here instead of three hours later in a GPU stage.
I could not inspect the built image from this host, so the exact package doing
the downgrade is not identified; the assert is there to make that assumption
self-checking.
Picks up the sglang-side fixes for three symbols v0.5.16 renamed or moved: DecodeCudaGraphRunner.num_tokens_per_bs -> captured_req_width, dp_attention.get_attention_tp_size -> ParallelState.attn_tp_size, and layers.utils.cp_utils -> layers.cp.padding. All eight completed stage-c failures in the previous run were the first two.
TpModelWorker.tp_rank -> self.ps.tp_rank, ModelRunner._model_update_group -> weight_updater._model_update_group, and iter_runners reaching draft_runner / draft_runner_list through the BaseSpecWorker.draft_worker property instead of off the *V2 wrapper. Round 4 got 3 stage-c jobs green (round 3: 0). The remaining three failures were these bugs, plus 'window_size_left' which is still entangled with them -- this run should separate the two.
A `for` loop exits with the status of its last iteration, so a patch that failed
to apply was silently swallowed as long as a later one succeeded. With two
patches, a failure in the alphabetically-first
`te_dequantized_backward_override.patch` left `grouped_linear.py` unpatched and
the image shipped anyway.
That matters right now because both remaining GPU failures are inside the two
files these patches touch:
ValueError: not enough values to unpack (expected 21, got 16)
transformer_engine/pytorch/module/grouped_linear.py:405
<- te_dequantized_backward_override.patch
TypeError: _flash_attn_forward() got an unexpected keyword argument
'window_size_left'
transformer_engine/pytorch/attention/dot_product_attention/...
<- te_fa2_sm103_whitelist.patch
Both jobs are green on main, so something under them changed with the v0.5.16
base image. This commit does not claim to fix that -- it makes the build say so
if a patch stopped applying, instead of producing an image that only fails hours
later inside a GPU stage.
…ge#24 Validates the TE 2.17 grouped-linear contract fix end to end. The `not enough values to unpack (expected 21, got 16)` failures in the test_glm5_*_lora_ci tests come from megatron-bridge's `_forward_te_grouped_linear` calling TE's private `_GroupedLinear` with TE <=2.14's positional layout; miles picked that up when it bumped TransformerEngine 2.12 -> 2.17 in #1781. Revert to `@bridge` -- ideally pinned to the merged commit rather than a branch, as line 82 does for mbridge -- once #24 lands. Note this does not address `window_size_left`, which is a separate TE 2.17 vs flash-attn skew inside TE's own context-parallel path.
TE 2.17 moved use_flash_attn_3 out of the window_size-tuple branch in cp_p2p_fwd/bwd_flash_attn, so it passes window_size_left/right to FA3. The flash_attn_3 wheel in miles-wheels was 3.0.0b1, whose _flash_attn_forward only accepts a window_size tuple, so every context-parallel path raises TypeError. TE has no FA3-only kill switch -- NVTE_FLASH_ATTN disables FA2 too. miles-wheels#10 rebuilt the wheel at flash-attention 00756db (3.0.0) and replaced the release asset, so the pip install here already picks it up. That wheel ships flash_attn_interface top-level and a flash_attn_3 re-export shim, making the separate interface fetch unnecessary -- and harmful, since it pinned a second revision that can drift from the one the extension module was built at, and overwriting the shim with the full module double-registers the flash_attn_3:: custom ops.
#1796 switched TransformerEngine to the rolling wheels installed with --no-deps. That pins the three TE dists to exactly those wheels, which is the point, but it also drops transformer_engine_torch's own runtime requirements: einops, onnx, onnxscript, pydantic, nvdlfw-inspect. transformer_engine.pytorch imports onnxscript unconditionally on its core import path (module/__init__ -> layernorm_linear -> base -> _common -> export -> onnx_extensions), so every image built since #1796 fails at ModuleNotFoundError: No module named 'onnxscript' as soon as anything touches TE -- which is every GPU test. verify_transformer_engine.py did not catch it because it only compared versions and metadata and called find_spec('transformer_engine'), which resolves the package directory without importing it. Check that every non-TE requirement of transformer_engine_torch is actually installed, so the build fails at the layer that introduced the gap instead of shipping a green image.
Megatron-Bridge#24's first revision only handled half of TE 2.17's grouped-linear contract change: m_splits was hoisted to a positional parameter *and* retyped to torch.Tensor. Passing the Python list made _GroupedLinear.forward raise 'list' object has no attribute 'tolist', which failed every LoRA test in stage-c. Pin by SHA rather than branch. buildkit keys this layer on the instruction text, so moving the branch would have left the old revision in the image with nothing to signal it.
radixark/Megatron-Bridge#24 landed on @bridge as 7f0fb345 (squash). Its src/megatron/bridge/peft/utils.py is byte-identical to 36d20288, the revision this PR already exercised, so the pin moves off the PR branch without changing what runs. Kept as a SHA rather than @bridge. Line 82 pins mbridge the same way, and the alternative is the failure this PR already hit once: buildkit keys the layer on the instruction text, so a moving branch silently reuses the cached layer and ships the previous revision.
TE 2.17's fused attention calls cudnn-frontend's
Reshape_attributes::set_reshape_mode, which sets the backend attribute
CUDNN_ATTR_OPERATION_RESHAPE_MODE. That attribute exists from cuDNN 9.22.0.
Against the pinned 9.16.0.29 it comes back BAD_PARAM and TE raises out of
fused_attn_bwd:
fused_attn_f16_arbitrary_seqlen.cu:934 cuDNN Error:
set_attribute(..., CUDNN_ATTR_OPERATION_RESHAPE_MODE, ...)
... CUDNN_STATUS_BAD_PARAM
It killed test_qwen3_5_35b_a3b_lora_ci (h100) and test_lora_qwen2.5_0.5B
(h200) in #1795, and reaches only configs where TE does not select FA3, so
most tests never touch it.
The old pin is obsolete in both of its purposes. It existed for
pytorch/pytorch#168167 (Conv3D bf16 collapse on H200), whose maintainers say
the fix landed in PyTorch 2.10 and whose stated workaround for 2.9.x was to
install a *newer* cuDNN, 9.15+. This image runs torch 2.11.0, and torch 2.11
already ships 9.19.0.56, so the pin now only drags cuDNN backwards.
Dropping the pin is not enough: 9.19.0.56 is still below 9.22.0. 9.22.0.52 is
the lowest release that satisfies TE, chosen over the newer 9.23-9.25 to keep
the change minimal.
The apt copy shadows the pip copy in ldconfig, and the loader interleaves the two: libcudnn_heuristic.so.9 comes from apt while libcudnn_graph.so.9 comes from pip. transformer_engine then fails either on import with an undefined symbol or inside fused attention with CUDNN_STATUS_BAD_PARAM, which is why moving the pip pin alone changed nothing. Verified on radixark/miles:pr-1795@sha256:792974ed with 8 DotProductAttention forward+backward configs (d=64/128 x MHA/GQA x sbhd/bshd): apt present + pip 9.16.0.29 -> torch refuses to init cuDNN apt present + pip 9.22.0.52 -> 8/8 fail apt removed + pip 9.19.0.56 -> 8/8 fail (the version floor is real) apt removed + pip 9.22.0.52 -> 8/8 pass
# Conflicts: # docker/Dockerfile
v0.5.16 moved should_allreduce_fusion/use_reduce_scatter out of the self.mlp(...) call args and into a surrounding get_forward().scoped(...) context manager, so the match text found 0 occurrences. source_patcher raises on a non-unique match, which killed the sglang scheduler during load_model and left test_dumper hanging to its 1800s timeout. The other seven matches in the sglang config still resolve uniquely against v0.5.16.
… shards Revert once the cuDNN BAD_PARAM in fused_attn_bwd is understood. A devbox on the same image digest runs the qwen2.5 LoRA test to completion through FusedAttention sub-backend 1 with zero cuDNN errors, so the difference is host-level and not recoverable from an ordinary CI log. Captures per shard: driver version and GPU state (including memory already in use by earlier tests in the same container), kernel, ldconfig/dpkg/pip cuDNN, the libcudnn actually mapped into a torch+TE process and its cudnnGetVersion(), the mounted model snapshot's config.json, ulimits, and the full container env. NVTE_DEBUG=2 is injected into the training process so TE logs the available and selected attention (sub-)backend per call.
f-string !r emitted literal backslash escapes, which exec_command then wrapped in another bash -c, so the probe died with a syntax error and took the test with it (qwen3.5 lora failed at 49s). Ship the script as base64 instead: the emitted command contains no quotes or newlines at all. Verified locally that the payload round-trips and that the decoded script passes bash -n.
Root cause found: the CI job's requirements resolve downgraded the image's cuDNN. Fixed in #1836, cherry-picked below.
The GPU jobs install requirements.txt inside the test container. torchft-nightly pulls torch, torch pins nvidia-cudnn-cu13 exactly, and that resolve uninstalls the cuDNN the image deliberately installed last: Collecting nvidia-cudnn-cu13==9.19.0.56 (from torch>=2.7->torchft-nightly->-r requirements.txt) Uninstalling nvidia-cudnn-cu13-9.22.0.52 Successfully installed nvidia-cudnn-cu13-9.19.0.56 transformer_engine 2.17's fused attention backward then fails with CUDNN_STATUS_BAD_PARAM on CUDNN_ATTR_OPERATION_RESHAPE_MODE, which is what has been failing test_lora_qwen2.5_0.5B, test_gpt_oss_20b_moe_lora_ci and test_qwen3_5_35b_a3b_lora_ci on #1795. Record the version the image shipped and reinstall it after the resolve, rather than hardcoding it here: the Dockerfile stays the single place the version is chosen, and --no-deps keeps the rest of the resolved set untouched. Declaring the pin in requirements.txt instead does not work -- pip resolves the conflict against torch's exact pin by downgrading torch itself to 2.10.0 and swapping the whole CUDA stack to cu12. Verified on an H200 devbox running the exact CI image digest: the requirements resolve reproduces the downgrade (9.22.0.52 -> 9.19.0.56, torch untouched), the restore puts 9.22.0.52 back, and TE fused attention backward then passes 8/8 configs that fail at 9.19.0.56. (cherry picked from commit bad0c0f)
GitHub runs 'shell: bash' as 'bash -e {0}'. '[ -n "$v" ] && pinned=...' as the last
statement of the loop body makes the loop exit non-zero whenever a package is absent
(cu12 on a cu13 image), which aborted the whole step and every GPU job with it. Use an
if-block so the loop always ends cleanly.
Verified by extracting the step from the workflow and running it under 'bash -e' with a
stub pip: exits 0 both when the version is untouched and when it has to be restored.
(cherry picked from commit 47ecd5d)
GitHub runs steps as 'bash --noprofile --norc -e -o pipefail'. 'pip show' exits 1 for a package that is not installed (cu12 on a cu13 image), pipefail propagates that through the awk pipe, and -e then aborts the step -- taking every GPU job with it. Terminate both command substitutions with '|| true', matching how this file already reads optional values out of the PR body. Verified by extracting the step from the workflow and running it under the exact shell line GitHub logs, with a stub pip where cu12 is absent: exits 0, and still restores the pin when the resolve moves it. (cherry picked from commit 20e30ce)
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
2 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.
Bumps the sglang base image and the sglang-miles branch to v0.5.16.
SGLANG_IMAGE_TAG:v0.5.15→v0.5.16(Dockerfile +build.pycu12-x86)SGLANG_BRANCH:sglang-miles→sglang-miles-v0.5.16Follows the v0.5.15 cycle (#1655 then #1701): this PR pins the versioned branch, and a
follow-up points
SGLANG_BRANCHback tosglang-milesonce it is promoted.sglang side
sglang-miles-v0.5.16rebases the 31 sglang-miles commits from v0.5.15 onto v0.5.16,landing as 22 commits: https://github.com/sgl-project/sglang/tree/sglang-miles-v0.5.16
Three PRs were dropped because v0.5.16 already carries them:
tp_worker.pyget_gradcallable,threshold_dsl.pyand its test are present (its two/pull_weightsfixups were kept)Notable reimplementation, because v0.5.16 refactored the same areas independently:
ModelRunner.weight_updater, so the spec-draft work(#27749/#28575/#27750) is reimplemented there: the receive/load split lands on
WeightUpdater, andSchedulerWeightUpdaterManagerowns the target-receives-once /load-into-each-runner fan-out. The worker-level
update_weights_from_*entry pointsare dropped so there is only one path.
RemoteInstanceWeightTransporter, so#21278's per-rank
RankParallelismConfigpublishing moved with it.#29874'sfree_lorais dropped: v0.5.16'sLoRAMemoryPool.remove_lora()alreadyreleases the slot on unload and additionally zeroes the buffers for cuda-graph replay.
#30421was extended to v0.5.16's newdeepseek_v4_dspark.py, which had the samestale
_attn_sink_localbug.get_global_server_args()call sites moved toruntime_context.get_server_args(); v0.5.16 ratchet-pins the legacy accessor to asingle call site and the test fails in both directions.
ci-sglang-pr: sglang-miles-v0.5.16
Test plan