[WideEP] Update DeepEP version in Dockerfile#45321
Conversation
|
Actually an issue here -- we need to update NCCL to > 2.30.4 during DeepEP compilation as well, not just rely on installing it afterwards like I thought we could |
a8cb6bd to
49fbfce
Compare
DeepEPv2 (ElasticBuffer API) requires NCCL >= 2.30.4 at both compile and runtime. NCCL backward compatibility is guaranteed by NVIDIA, so installing a newer NCCL after PyTorch is safe. - Pin NCCL 2.30.4-1+cuda13.2 in the base build stage so the extensions-build stage inherits it for DeepEP compilation - Pin the same version in the vllm-base runtime stage, replacing the dynamic apt-cache madison lookup - Update DEEPEP_COMMIT_HASH from 73b6ea4 to d4f41e4e93 to pick up DeepEPv2 Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
2de2b77 to
71604cb
Compare
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
The CUDA base image holds NCCL packages, so installing them in the base stage without --allow-change-held-packages fails. Move NCCL installation to the extensions-build stage where it's actually needed for DeepEP compilation, with the correct apt flag. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
The apt libnccl-dev package only ships host API headers (nccl.h), not the GIN device headers (ncclGinRequest_t etc.) that DeepEPv2 compiles against. Install nvidia-nccl-cu13 via pip instead, which includes the full header set and is auto-detected by DeepEP's build system via EP_NCCL_ROOT_DIR. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
- Pin NCCL_VERSION to exactly 2.30.7 - Guard pip install (extensions-build) and apt install (vllm-base) behind CUDA major >= 13 checks - CUDA 12 builds fall back to whatever NCCL the base image ships Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
The previous approach failed because: 1. install_python_libraries.sh runs `uv pip install torch` which re-pins nvidia-nccl-cu13==2.28.9, overwriting our 2.30.7 2. apt libnccl-dev only has host API headers, not GIN device headers Fix both by: - Adding NCCL re-install to install_python_libraries.sh after torch, gated on NCCL_VERSION env var and CUDA >= 13 - Replacing apt NCCL install in vllm-base with pip install after all wheels are installed - Removing separate pre-build NCCL RUN from extensions-build Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
The test stage installs requirements/dev.txt which includes requirements/test/cuda.txt pinning nvidia-nccl-cu13==2.28.9, overwriting the 2.30.7 upgrade from vllm-base. Add another NCCL upgrade after all test dependencies are installed. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
Replace the three separate `uv pip install nvidia-nccl-cu13==2.30.7 --no-deps` commands (in install script, vllm-base, and test stage) with a single UV_OVERRIDE env var that forces uv to resolve nvidia-nccl-cu13 to our pinned version whenever it appears as a transitive dependency. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
Add instructions for pinning NCCL >= 2.30.4 when installing vLLM via uv/pip rather than the Docker image, covering both UV_OVERRIDE and pip --no-deps approaches. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
|
Documentation preview: https://vllm--45321.org.readthedocs.build/en/45321/ |
| ldflags = ["-lnccl"] | ||
| nccl_lib_paths = find_nccl_library_paths() | ||
| if nccl_lib_paths: | ||
| ldflags = [f"-L{p}" for p in nccl_lib_paths] + ldflags |
There was a problem hiding this comment.
The NCCL symm mem allocator JIT-compiles a C++ extension that links with -lnccl. Previously the apt libnccl-dev package placed libnccl.so in /usr/lib where the system linker could find it. With pip-only NCCL, the .so is in site-packages/nvidia/nccl/lib/ — invisible to ld.
This adds find_nccl_library_paths() to discover the pip package's lib dir and passes -L<path> to the linker, fixing the cannot find -lnccl error without needing apt.
— via claude
|
Hi @tlrmchlsmth, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
The NCCL symmetric memory allocator JIT-compiles a C++ extension with `-lnccl`, but the system linker can't find `libnccl.so` when NCCL is installed only via pip (in site-packages/nvidia/nccl/lib/). Add `find_nccl_library_paths()` to discover the pip package's lib directory and pass `-L<path>` to the linker. This was a pre-existing issue masked by the apt `libnccl-dev` install; removing apt NCCL in favor of pip-only exposed it. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
78c38b5 to
11feadc
Compare
pytest.skip() raises a BaseException that bypasses the except-Exception handler in _worker_parallel_launch, causing torch.multiprocessing.spawn workers to SIGSEGV instead of cleanly skipping. Raise a regular GINNotAvailableError inside the subprocess and convert it to pytest.skip() in parallel_launch on the parent side. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
|
Hi @tlrmchlsmth, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
Temporary diagnostic to determine why ginType == NONE on B200-k8s CI pods: checks NIC model, OFED version, nvidia-peermem, kernel version, RDMA device access, and NCCL env vars. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
query_nccl_gin_type() returns None when it can't query (e.g. _comm_ptr missing, ncclCommQueryProperties unavailable). The previous check only guarded against ginType==0, allowing None to pass through and hit the ElasticBuffer segfault. Now None is treated as GIN-unavailable too. Also promote debug logs to warning so failures are visible in CI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
The nvidia-nccl-cu* pip package ships libnccl.so.2 (runtime) but not the libnccl.so dev symlink. The nccl_allocator JIT extension needs -lnccl at link time. The previous apt-based libnccl-dev install provided this symlink; now that we use the pip package instead, we create it explicitly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
Use -l:libnccl.so.2 instead of -lnccl so the JIT extension links against the SONAME directly. This removes the need for the libnccl.so dev symlink that the pip nvidia-nccl-cu* package doesn't ship, and that libnccl-dev (apt) would otherwise need to provide. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
The lifecycle test now runs because NCCL >= 2.30.4 is present, but on pods without IBGDA-capable hardware it hits the GIN RuntimeError from _check_gin_support. Detect this in _spawn_workers and convert to pytest.skip. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
- Remove debug logger.info for GIN type (was temporary instrumentation) - Remove NCCL GIN diagnostics echo from B200 CI step - Add comment explaining why GIN check happens post-spawn Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
NCCL validates magic, version, and size on the ncclCommProperties struct (same pattern as NCCL_CONFIG_INITIALIZER). Without these, ncclCommQueryProperties rejects the struct. Co-authored-by: Ilya Markov <ilmarkov@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
|
Hi @tlrmchlsmth, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
The NCCL communicator is lazily created on the first collective. When _check_gin_support runs during process_weights_after_loading, _comm_ptr() returns 0 (NULL) and the query fails, incorrectly blocking startup. Fix: return None from query_nccl_gin_type when comm_ptr is 0, and retry the check on subsequent get_handle calls once the comm exists after real collectives. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
c6e3c95 to
0f698a0
Compare
`pip install tensorizer` resolves torch's transitive dep nvidia-nccl-cu13==2.28.9, downgrading the 2.30.7 installed by the Docker image. This breaks deep_ep import (undefined symbol: ncclCommQueryProperties). Switch to `uv pip install` so the UV_OVERRIDE NCCL pin is respected. Also: remove unused _NCCL_GIN_TYPE_NONE constant, improve GIN check comment in test utils. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
uv pip install fails on AMD CI where no virtual environment exists. Switch to pip install --no-deps to avoid both the venv requirement and the risk of torch pulling in an older NCCL as a transitive dependency. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tyler Michael Smith <tyler@vllm.ai> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
|
Hi @tlrmchlsmth, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
Update DeepEP to deepseek-ai/DeepEP@d4f41e4 so that we can pick up DeepEPv2.
Missed this in #41183
Ideally we'd have a single source of truth for the commit across both the install script and the Dockerfile but I don't see a sane way to do it at the moment
PyTorch ships an earlier version of NCCL. Per feedback from the NCCL team, I think this should be safe as
For the pypi wheel, no change is needed since we don't ship DeepEP in the wheel.