Skip to content

[fix] Build DeepGEMM against selected torch - #2139

Open
hershg wants to merge 1 commit into
NovaSky-AI:mainfrom
hershg:fix/torch-compatible-deepgemm
Open

[fix] Build DeepGEMM against selected torch#2139
hershg wants to merge 1 commit into
NovaSky-AI:mainfrom
hershg:fix/torch-compatible-deepgemm

Conversation

@hershg

@hershg hershg commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • SkyRL selects Torch 2.11, while vLLM 0.28 bundles a DeepGEMM extension linked against Torch 2.13. GLM 5.3 sparse-attention startup therefore fails with an undefined c10 symbol when vLLM imports its vendored extension.
  • Add vLLM 0.28’s exact DeepGEMM revision to the Linux Megatron extra and force-build it against SkyRL’s selected Torch. vLLM then prefers the compatible external package.
  • Keep the dependency out of the FSDP extra: FSDP does not need this GLM-specific CUDA extension, and installing it on CPU-only hosts requires CUDA_HOME.

This changes no Torch, vLLM, FlashInfer, or CUDA version and adds no runtime monkey patch.

Failure reproduced

On SkyRL 955c3e23, Torch 2.11.0+cu130, and vLLM 0.28.0, importing vllm.third_party.deep_gemm._C failed with an undefined c10::ValueError symbol. GLM 5.3 TP8 startup then failed because its sparse-attention indexer requires DeepGEMM. Rebuilding revision 8b1392b978f5a03c828dd1711090d7fb50958b8a against the selected Torch made the extension importable; deep_gemm.get_num_sms() returned all 148 B300 SMs and vLLM reported DeepGEMM support enabled.

Validation

  • uv lock --check
  • check_code_quality, skyrl_gym_tests, skyrl_train_tests, and the full skyrl_tests suite pass on the final commit.
  • Exact dependency-equivalent CUDA 13 image: one-node B300 TP8 GLM 5.3 BF16 startup and bounded generation passed.
  • Two-node B300 TP8 inference + TP8/EP8 policy, rank-32 parity passed in 36m18s: initialized mean logprob difference 0.031523; deliberate-update effect 0.028250; updated mean difference 0.031979; acceptance threshold 0.05.
  • TCLI XID 1049812 passed 4×4×1 end to end on the same image: 16/16 rollouts, real forward/backward, optimizer step, finite gradient/KL/ESS/log-ratio metrics, durable checkpoints 0 and 1, and clean scheduler release.

The repository GPU workflow did not launch a remote job because the PR context had no ANYSCALE_CLI_TOKEN; it failed at credential validation before GPU setup or tests. The B300 receipts above cover the extension, GLM startup, parity, and real training path.

Scope and risk

The package is Linux/x86_64 and Megatron-only. It adds a source build during environment creation, so image build time increases. Runtime behavior outside consumers that import DeepGEMM is unchanged.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request integrates the external deep-gemm package into the project's dependencies and CI pipeline, building it against the selected PyTorch runtime to resolve compatibility issues with vLLM's vendored extension. It updates pyproject.toml with the new dependency, build configurations, and source repository, adds a new test to verify that deep_gemm loads correctly, and updates the documentation accordingly. Feedback suggests using pytest.importorskip in the new test to gracefully skip it in environments where deep-gemm is not installed, preventing unexpected test failures.

Comment thread tests/backends/skyrl_train/gpu/gpu_ci/test_deep_gemm.py
@hershg

hershg commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

CI note: skyrl_gpu_tests failed before submitting any GPU work because the fork-triggered workflow had no valid Anyscale credentials (run). The patch itself was tested from fresh isolated megatron and fsdp environments on B300; both passed, and the Megatron environment built DeepGEMM from source. The Vercel failure is likewise authorization-only.

Signed-off-by: Hersh Godse <hersh@trajectory.ai>
@hershg
hershg force-pushed the fix/torch-compatible-deepgemm branch from 18eb761 to 1b23829 Compare September 2, 2026 13:50
@hershg

hershg commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: the first revision added the CUDA-built package to both fsdp and megatron, which made the ordinary CPU FSDP test install fail before pytest because that runner has no CUDA_HOME. Commit 1b238298 narrows DeepGEMM to the Megatron extra—the backend exercised by the GLM sparse-indexer repro—and removes the unrelated FSDP GPU-test invocation. uv lock --check, git diff --check, and shell syntax validation pass. The B300 Megatron import/kernel, TP8 startup, and two-node GLM LoRA parity receipts remain applicable because this does not change the Megatron environment.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 1b23829. Configure here.

Comment thread pyproject.toml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant