Skip to content

build: enable multi-node HybridEP in container images - #4038

Merged
terrykong merged 10 commits into
NVIDIA-NeMo:mainfrom
seonjinn:sna/hybridep-multinode-image-20260906
Sep 9, 2026
Merged

terrykong merged 10 commits into
NVIDIA-NeMo:mainfrom
seonjinn:sna/hybridep-multinode-image-20260906

Conversation

@seonjinn

@seonjinn seonjinn commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Build DeepEP with HYBRID_EP_MULTINODE=1 in both production Dockerfiles.
  • Include HYBRID_EP_MULTINODE in the uv seed compatibility key so stale single-node wheels are skipped without deleting reusable multi-node wheels.
  • Leave docker/install_apptainer.sh unchanged because it installs Apptainer and does not build DeepEP.

Why

DeepEP reads HYBRID_EP_MULTINODE while building its extension. Setting the variable only at runtime cannot add inter-node transport support to an existing single-node wheel.

This change also enables multi-node AutoModel HybridEP recipes proposed in #4037.

Testing

  • Docker HybridEP build configuration test: 1 passed.
  • ruff check
  • ruff format --check
  • docker buildx build --check for docker/Dockerfile
  • docker buildx build --check for docker/Dockerfile.ngc_pytorch
  • git diff --check

A full image build and multi-node runtime validation will be recorded separately.

Signed-off-by: seonjinn <sna@nvidia.com>
@seonjinn
seonjinn requested review from a team as code owners September 7, 2026 04:34
@copy-pr-bot

copy-pr-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@seonjinn seonjinn added the CI:L0 Run doctests and unit tests label Sep 7, 2026
@seonjinn

seonjinn commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 116ac87

sharonyu-115
sharonyu-115 previously approved these changes Sep 7, 2026

@sharonyu-115 sharonyu-115 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.

LGTM. Thanks!

Signed-off-by: seonjinn <sna@nvidia.com>
@seonjinn

seonjinn commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test ff04168

Signed-off-by: seonjinn <sna@nvidia.com>
Signed-off-by: seonjinn <sna@nvidia.com>
@seonjinn

seonjinn commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 67f7159

1 similar comment
@seonjinn

seonjinn commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 67f7159

@seonjinn
seonjinn requested a review from yuki-97 September 7, 2026 07:44
Signed-off-by: seonjinn <sna@nvidia.com>
@seonjinn

seonjinn commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 8dde640

@seonjinn
seonjinn requested a review from terrykong September 8, 2026 00:55
Comment thread docker/Dockerfile
Comment thread docker/Dockerfile
Comment thread tests/unit/tools/test_docker_hybridep_build.py Outdated
Signed-off-by: seonjinn <sna@nvidia.com>
Signed-off-by: seonjinn <sna@nvidia.com>
@seonjinn

seonjinn commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 36275f4

Signed-off-by: seonjinn <sna@nvidia.com>
@terrykong
terrykong enabled auto-merge (squash) September 9, 2026 05:44
@seonjinn

seonjinn commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 87151f9

@kajalj22
kajalj22 self-requested a review September 9, 2026 05:57
kajalj22
kajalj22 previously approved these changes Sep 9, 2026
Signed-off-by: seonjinn <sna@nvidia.com>

# Conflicts:
#	docker/Dockerfile
@seonjinn

seonjinn commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 28f4927

@seonjinn

seonjinn commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test e21d91e

@terrykong
terrykong merged commit e518e60 into NVIDIA-NeMo:main Sep 9, 2026
65 of 67 checks passed

@seonjinn seonjinn left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Four notes on the NVML apt bracket and the DeepEP cache clean, plus a test diagnostics nit. Nothing blocking - the purge is in-layer so the shipped image is clean.

Comment thread docker/Dockerfile
# Everything is hardlinked against the uv cache; this layer holds exactly one real copy of every wheel.
# The venv is symlinked to avoid bloating the layer size
apt-get update
apt-get install -y --no-install-recommends libnvidia-ml-dev

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

libnvidia-ml-dev is not a stub install. On this PR's CI build apt pulled 7 packages, 136 MB download / 451 MB on disk, because DeepEP links with -l:libnvidia-ml.so.1 (setup.py:143) and that exact .so.1 ships only in libnvidia-compute-580, which the -dev package depends on.

It stays installed from #131 3.7s to #131 1950.6s, roughly 25 min after deep-ep finishes linking at 452.4s, so it also spans the sglang/mcore/automodel/modelopt/all-groups syncs. The purge and autoremove are in the same RUN, so the shipped image is provably clean (450 MB freed at #131 1953.1) - this is build hygiene, not a ship risk.

Worth checking whether symlinking the CUDA NVML stub to libnvidia-ml.so.1 and setting LIBRARY_PATH satisfies the -l: link, which would avoid the 451 MB entirely. Also note the package resolves to 12.0.140~12.0.1-4build4 from noble/multiverse inside a cuda-dl-base:26.05-cuda13.2 image.

Comment thread docker/Dockerfile
# The venv is symlinked to avoid bloating the layer size
apt-get update
apt-get install -y --no-install-recommends libnvidia-ml-dev
uv cache clean deep-ep

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

uv cache clean deep-ep runs after the seed rsync at L220, so it evicts the seeded wheel on every build, not once.

It was a no-op on this run (#131 11.60 No cache entries found - the seed was skipped on a cache-key mismatch), but once the :uv-cache tag is refreshed on main this becomes a recurring ~395s rebuild (Building deep-ep at 57.7s to Built at 452.4s, inside a 1958s layer that gates every downstream CI job).

uv already tracks this for you: [tool.uv.extra-build-variables] (pyproject.toml:542) participates in the build cache key, so changes to it trigger a rebuild of just the affected package. Adding

deep_ep = { HYBRID_EP_MULTINODE = "1" }

invalidates once, then stays cached, and makes the flag authoritative for any uv sync rather than only these two Dockerfiles. Keep ENV HYBRID_EP_MULTINODE=1 for runtime JIT.

Please don't fold this into CACHE_KEY instead - L215 skips the entire seed on mismatch, so that would rebuild every source package to invalidate one wheel.

Side note: the mirrored clean at Dockerfile.ngc_pytorch:123 is always a no-op. That stage sets UV_CACHE_DIR=/opt/uv_cache (L89) with no seed stage and no cache mount, so the cache is empty when it runs.

uv venv --system-site-packages ${UV_PROJECT_ENVIRONMENT}
uv pip install --no-cache-dir --no-deps /tmp/build_vllm/vllm/vllm*.whl
# Ensure nvshmem is installed before building DeepEP
apt-get update

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

# Ensure nvshmem is installed before building DeepEP (L120) used to sit directly above the uv sync that pulls nvidia-nvshmem-cu13 (pyproject.toml:62). These apt lines were inserted between them, so the comment now reads as if libnvidia-ml-dev provides nvshmem - it doesn't, that's NVML.

Suggest moving the nvshmem comment back down to L124, and adding a separate comment above the apt block explaining why NVML headers are needed for the multi-node DeepEP build. That rationale isn't in the PR description either, which still only mentions the env var and the cache clean.

cache_clean = "uv cache clean deep-ep"
nvml_stub_purge = "apt-get purge -y libnvidia-ml-dev"
nvml_stub_dependencies_purge = "apt-get autoremove -y"
nvml_stub_purge_index = lines.index(nvml_stub_purge)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nvml_stub_purge_index = lines.index(nvml_stub_purge) runs before the membership asserts, which makes the assert nvml_stub_purge in lines, ... below it dead code - it can never fail. Remove the purge line from a Dockerfile and you get ValueError: 'list.index(x): x not in list' here instead of the message you wrote (Python 3.11+ omits the value from that error, so the traceback is genuinely opaque).

Same for first_sync_index = next(...) at L45 with no default: a missing uv sync raises a bare StopIteration rather than the ordering assert's message.

Moving both lookups below the membership asserts, and using next(..., None) plus an explicit assert for first_sync_index - the pattern you already use for nvml_stub_dependencies_purge_index - fixes both.

Minor: nvml_stub_* is a misnomer. libnvidia-ml-dev pulls in the real driver library, not a stub.

yuekaizhang added a commit to yuekaizhang/RL that referenced this pull request Sep 18, 2026
Plain "deepep" hangs with "DeepEP error: timeout (dispatch CPU)" on this
EP=32, 4-node/32-rank layout under the deep_ep pin bumped by
NVIDIA-NeMo#3436 (deep_ep 29d31c0 -> 17cfb81, "enable HybridEP on
H100/B200"). The container's deep_ep is already built with
HYBRID_EP_MULTINODE=1 (NVIDIA-NeMo#4038, merged), and "hybridep" is
already a supported nemo_automodel BackendConfig.dispatcher value -- just
not yet the repo-wide default for AutoModel MoE recipes (that's
NVIDIA-NeMo#4037, still open). Switching these two recipes now rather
than waiting on NVIDIA-NeMo#4037: verified with a live 4n8g eagle3 run past the exact
point (policy.get_logprobs()'s first post-refit call) that failed twice
under plain deepep.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Yuekai Zhang <zhangyuekai@foxmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:L0 Run doctests and unit tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants