Skip to content

[Model][Engram] Share host tables across co-located DP replicas by default - #57651

Merged
ZJY0516 merged 4 commits into
vllm-project:mainfrom
Juntian777:feat/engram_dp_shared_memory_default
Sep 21, 2026
Merged

ZJY0516 merged 4 commits into
vllm-project:mainfrom
Juntian777:feat/engram_dp_shared_memory_default

Conversation

@Juntian777

Copy link
Copy Markdown
Contributor

Purpose

EngramConfig.dp_shared_memory (#56512) keeps one copy of the CPU-offloaded Engram tables per node instead of one per DP replica, with no per-step Engram DP collectives, but it had to be requested by hand and failed startup whenever its requirements were not met. This PR turns it on by default wherever it can work and falls back otherwise:

  • dp_shared_memory defaults to unset and resolves to enabled when cpu_offload is on, data_parallel_size > 1 and elastic EP is off. An explicit true keeps the existing strict validation.
  • Before the layers are built, can_share_engram_tables falls back to the existing DP head sharding (same host footprint, one table per node) when the node-local Engram DP group has size 1 (replicas not co-located) or /dev/shm is smaller than the full tables: 188.8 GiB for DeepSeek-V4.1-Flash, against Docker's default 64 MiB. Both cases log a warning_once pointing at --shm-size / --ipc=host. The check compares against the tmpfs total, which is identical on every rank of a node, so no collective is needed to keep the decision consistent.
  • The load_format allowlist is dropped: dummy (fill_) and sharded_state (copy_) write identical bytes into the shared mapping on every rank, so the restriction guarded nothing.

Duplicate-work check (2026-09-19): #57195 shares host lookups for SP token owners, #56757 backs tables with disk files, #57491 keeps tables in host memory on ROCm. None changes the dp_shared_memory default or adds a /dev/shm fallback.

Test plan

.venv/bin/python -m pytest tests/distributed/test_engram_dp_shard.py \
  tests/test_config.py -q -k engram
.venv/bin/pre-commit run --files vllm/config/engram.py vllm/config/vllm.py \
  vllm/models/deepseek_v41/nvidia/engram.py vllm/models/deepseek_v41/nvidia/model.py \
  tests/test_config.py tests/distributed/test_engram_dp_shard.py
.venv/bin/pre-commit run mypy-3.12 --hook-stage manual --files vllm/config/engram.py \
  vllm/config/vllm.py vllm/models/deepseek_v41/nvidia/engram.py \
  vllm/models/deepseek_v41/nvidia/model.py
git diff --check

Test result

  • 68 passed on 4xGB200, including the four-process shared-memory storage tests, the new default-resolution cases (offloaded DP on; no offload, DP=1 and elastic EP off) and the new /dev/shm-too-small fallback test.
  • Pre-commit, mypy-3.12 and git diff --check passed.

Model evaluation

DeepSeek-V4.1-Flash (dba1be0a), one GB200 node, DEP4 (TP1 x DP4, EP), DSpark 5 tokens with adaptive verification, no --engram-config flag. GSM8K 5-shot via MQA, completions API, temperature=0, max_tokens=1024, n=1319.

Metric Value
exact_match/flexible-extract 0.931
exact_match/strict-match 0.931
num_request_errors 0

This matches the 0.9303 recorded for the same model on main (2026-09-13). The engine log confirms the default took the shared path on its own: Resolved Engram configuration: EngramConfig(cpu_offload=True, embedding_across_dp=False, dp_shared_memory=True), Engram node-local DP group size: 4 (TP=1), and both tables reported 94.42 GiB shared across DP replicas with no fallback warning. On the node, /dev/shm held 190 GiB and free reported 191 GiB shared, one copy of the tables rather than four private pinned copies.

AI assistance was used for implementation, testing, and this draft (Claude Code). Human review of every changed line and independent execution of relevant tests are required before submission.

🤖 Generated with Claude Code

…fault

dp_shared_memory now defaults to unset and resolves to enabled whenever
cpu_offload is on, data_parallel_size > 1 and elastic EP is off. Before
the layers are built, the model falls back to DP head sharding when the
DP replicas are not co-located on one node or /dev/shm is smaller than
the full tables, so a Docker default 64 MiB shm no longer fails startup.
The load_format allowlist is dropped: dummy and sharded_state loaders
write identical bytes into the shared mapping on every rank.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Juntian Liu <Juntianl777@gmail.com>

@claude claude 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added deepseek Related to DeepSeek models DSv4.1 Related to DeepSeek-V4.1 models labels Sep 19, 2026
@mergify

mergify Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @Juntian777.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Sep 21, 2026
Signed-off-by: zjy0516 <riverclouds.zhu@qq.com>
@ZJY0516 ZJY0516 added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 21, 2026
@ZJY0516

ZJY0516 commented Sep 21, 2026

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

✅ @Juntian777, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • Your branch must contain every commit currently on its upstream target branch. Merge or rebase onto the latest target branch, then rerun the command. Append --allow-stale to a run command to test an outdated branch at your own risk.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #90149 for commit d78f21af6bdf.

Juntian777 and others added 2 commits September 21, 2026 06:37
…m config

dp_shared_memory now defaults to None so the engine can resolve it, but
test_engram_config_cli_optional still asserted False for
`--engram-config {}` and failed on `None is False`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Juntian Liu <Juntianl777@gmail.com>
@Juntian777

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #90161 for commit e2b0f6b93bc6.

@ZJY0516
ZJY0516 enabled auto-merge (squash) September 21, 2026 07:56
@ZJY0516
ZJY0516 merged commit 76ffb0d into vllm-project:main Sep 21, 2026
153 checks passed
gcanlin added a commit to gcanlin/vllm that referenced this pull request Sep 21, 2026
Resolves conflicts with vllm-project#57874 (can_use_mega_mhc) and vllm-project#57651 (engram
table sharing import) against the typed Mega-mHC FP8 outputs.

Signed-off-by: Canlin <canlinguosdu@gmail.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deepseek Related to DeepSeek models DSv4.1 Related to DeepSeek-V4.1 models needs-rebase ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants