refactor: move npu context parallel partition into model side. - #2003
Merged
Conversation
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
shifengmin
added a commit
to shifengmin/xllm
that referenced
this pull request
Jul 23, 2026
Model-side CP already restores global-real hidden before the LM head (npu_cp::gather_restore after the last decoder layer). PR xLLM-AI#2003 cleared outputHidden but left contextParallelInfo = ATTN_CP enabled, so the ATB LmHead graph still built a nested prefill CP AllGather on the already- global hidden (gated by gatherAhead && contextParallelInfo.IsEnabled(), independent of outputHidden). That second AllGather treated the full sequence as a CP shard and scaled hidden by cp_size again, peaking at O(cp_size * T * H) and OOM'ing on long prefill (e.g. cp=32, T~30000, H=6144, bf16 -> ~10.99 GiB). Drop the ATTN_CP assignment so contextParallelInfo stays default- constructed (rankIds empty -> IsEnabled()==false) and the ATB LmHead graph skips the CP AllGather node. gatherAhead (prefill indices Gather) and ATTN_TP (vocab column parallel) are unchanged. See docs/pr2003_lmhead_nested_cp_allgather_bug.md. Co-authored-by: Cursor <cursoragent@cursor.com>
shifengmin
added a commit
to shifengmin/xllm
that referenced
this pull request
Jul 23, 2026
Model-side CP already restores global-real hidden before the LM head (npu_cp::gather_restore after the last decoder layer). PR xLLM-AI#2003 cleared outputHidden but left contextParallelInfo = ATTN_CP enabled, so the ATB LmHead graph still built a nested prefill CP AllGather on the already- global hidden (gated by gatherAhead && contextParallelInfo.IsEnabled(), independent of outputHidden). That second AllGather treated the full sequence as a CP shard and scaled hidden by cp_size again, peaking at O(cp_size * T * H) and OOM'ing on long prefill (e.g. cp=32, T~30000, H=6144, bf16 -> ~10.99 GiB). Drop the ATTN_CP assignment so contextParallelInfo stays default- constructed (rankIds empty -> IsEnabled()==false) and the ATB LmHead graph skips the CP AllGather node. gatherAhead (prefill indices Gather) and ATTN_TP (vocab column parallel) are unchanged. See docs/pr2003_lmhead_nested_cp_allgather_bug.md. Co-authored-by: Cursor <cursoragent@cursor.com>
shifengmin
force-pushed
the
feat/npu-cp-model-closure
branch
from
July 23, 2026 05:50
6fd3e60 to
7f34a96
Compare
shifengmin
marked this pull request as ready for review
July 24, 2026 02:43
shifengmin
requested review from
Clement-Wang26,
DongheJin,
DragonFive,
JimHsiung,
Kang-Meng,
RobbieLeung,
XuZhang99,
liujinguang0125,
liutongxuan,
walsonyang,
xiao-yu-chen,
yingxudeng,
yq33victor and
zhang-minchao
as code owners
July 24, 2026 02:43
shifengmin
force-pushed
the
feat/npu-cp-model-closure
branch
from
July 24, 2026 02:44
7f28e51 to
77c9d4d
Compare
shifengmin
added a commit
to shifengmin/xllm
that referenced
this pull request
Jul 24, 2026
Model-side CP already restores global-real hidden before the LM head (npu_cp::gather_restore after the last decoder layer). PR xLLM-AI#2003 cleared outputHidden but left contextParallelInfo = ATTN_CP enabled, so the ATB LmHead graph still built a nested prefill CP AllGather on the already- global hidden (gated by gatherAhead && contextParallelInfo.IsEnabled(), independent of outputHidden). That second AllGather treated the full sequence as a CP shard and scaled hidden by cp_size again, peaking at O(cp_size * T * H) and OOM'ing on long prefill (e.g. cp=32, T~30000, H=6144, bf16 -> ~10.99 GiB). Drop the ATTN_CP assignment so contextParallelInfo stays default- constructed (rankIds empty -> IsEnabled()==false) and the ATB LmHead graph skips the CP AllGather node. gatherAhead (prefill indices Gather) and ATTN_TP (vocab column parallel) are unchanged. See docs/pr2003_lmhead_nested_cp_allgather_bug.md. Co-authored-by: Cursor <cursoragent@cursor.com>
shifengmin
force-pushed
the
feat/npu-cp-model-closure
branch
from
July 24, 2026 08:40
77c9d4d to
f6cf582
Compare
shifengmin
added a commit
to shifengmin/xllm
that referenced
this pull request
Jul 27, 2026
Model-side CP already restores global-real hidden before the LM head (npu_cp::gather_restore after the last decoder layer). PR xLLM-AI#2003 cleared outputHidden but left contextParallelInfo = ATTN_CP enabled, so the ATB LmHead graph still built a nested prefill CP AllGather on the already- global hidden (gated by gatherAhead && contextParallelInfo.IsEnabled(), independent of outputHidden). That second AllGather treated the full sequence as a CP shard and scaled hidden by cp_size again, peaking at O(cp_size * T * H) and OOM'ing on long prefill (e.g. cp=32, T~30000, H=6144, bf16 -> ~10.99 GiB). Drop the ATTN_CP assignment so contextParallelInfo stays default- constructed (rankIds empty -> IsEnabled()==false) and the ATB LmHead graph skips the CP AllGather node. gatherAhead (prefill indices Gather) and ATTN_TP (vocab column parallel) are unchanged. See docs/pr2003_lmhead_nested_cp_allgather_bug.md. Co-authored-by: Cursor <cursoragent@cursor.com>
shifengmin
force-pushed
the
feat/npu-cp-model-closure
branch
from
July 27, 2026 06:35
948c9ba to
3383cfe
Compare
shifengmin
added a commit
to shifengmin/xllm
that referenced
this pull request
Jul 27, 2026
Model-side CP already restores global-real hidden before the LM head (npu_cp::gather_restore after the last decoder layer). PR xLLM-AI#2003 cleared outputHidden but left contextParallelInfo = ATTN_CP enabled, so the ATB LmHead graph still built a nested prefill CP AllGather on the already- global hidden (gated by gatherAhead && contextParallelInfo.IsEnabled(), independent of outputHidden). That second AllGather treated the full sequence as a CP shard and scaled hidden by cp_size again, peaking at O(cp_size * T * H) and OOM'ing on long prefill (e.g. cp=32, T~30000, H=6144, bf16 -> ~10.99 GiB). Drop the ATTN_CP assignment so contextParallelInfo stays default- constructed (rankIds empty -> IsEnabled()==false) and the ATB LmHead graph skips the CP AllGather node. gatherAhead (prefill indices Gather) and ATTN_TP (vocab column parallel) are unchanged. See docs/pr2003_lmhead_nested_cp_allgather_bug.md. Co-authored-by: Cursor <cursoragent@cursor.com>
shifengmin
force-pushed
the
feat/npu-cp-model-closure
branch
from
July 27, 2026 09:10
b1c0c93 to
5b91ef9
Compare
- Move input padding and output reorder to after embedding and after the last decoder layer via a model-side CP closure (npu_cp_closure.h), so the scheduler, worker output processing, and MTP no longer special-case CP. - Delete the legacy worker-side cp_input_partition and its test (Phase E). - Fix SFA prev/next split to use local_padded seq lens so short or unaligned prompts no longer hit c10::IndexError; clamp gather indices defensively. - Loosen NPU validate_model_cp to allow tp_size>1 with dp_size==1 (CP x TP orthogonal), requiring npu_kernel_backend=ATB and enable_graph=false. - Isolate TP1 draft CP plan/tensors/slots from the target CP group, and route DeepSeek V3.2 CP draft to deepseek_v32_mtp. - Add unit tests for cp_group_ranks, npu_cp_capability, npu_cp_closure, ep_padding_plan, and prefill_plan. Co-authored-by: Cursor <cursoragent@cursor.com>
Model-side CP already restores global-real hidden before the LM head (npu_cp::gather_restore after the last decoder layer). PR xLLM-AI#2003 cleared outputHidden but left contextParallelInfo = ATTN_CP enabled, so the ATB LmHead graph still built a nested prefill CP AllGather on the already- global hidden (gated by gatherAhead && contextParallelInfo.IsEnabled(), independent of outputHidden). That second AllGather treated the full sequence as a CP shard and scaled hidden by cp_size again, peaking at O(cp_size * T * H) and OOM'ing on long prefill (e.g. cp=32, T~30000, H=6144, bf16 -> ~10.99 GiB). Drop the ATTN_CP assignment so contextParallelInfo stays default- constructed (rankIds empty -> IsEnabled()==false) and the ATB LmHead graph skips the CP AllGather node. gatherAhead (prefill indices Gather) and ATTN_TP (vocab column parallel) are unchanged. See docs/pr2003_lmhead_nested_cp_allgather_bug.md. Co-authored-by: Cursor <cursoragent@cursor.com>
The ATB-facing attention metadata (q_seq_lens / kv_seq_lens / q_cu_seq_lens / q_max_seq_len / kv_max_seq_len) must be built from local_padded_seq_lens so the lengths stay consistent with the hidden row count (local_padded_token_num), CpEpPadding, and SFA actual_seq_lengths. The legacy code used local_real, which mismatched for non-aligned prompts where local_real < local_padded: the ATB graph sized AllGather / attention buffers by kv_seq_lens (real) while the actual KV data held local_padded rows, causing SDMA DDR out-of-range crashes and garbled output. local_real is retained in the plan for restore / sampling / logging but must not be written into the decoder variant pack. Add CPU comparison unit tests (npu_cp_prefill_plan_cmp_test, npu_cp_ep_padding_cmp_test, npu_cp_slots_cmp_test) covering the prefill plan, EP padding, and slots / gather-index computation under chunked prefill + prefix cache + kvsplit. Co-authored-by: Cursor <cursoragent@cursor.com>
…boundary. Model-side CP localizes hidden after embedding and restores global-real hidden before the lm head, so both layers always see the full sequence and must be CP-unaware but DP-aware. Previously they sharded by the CP-local TP (size tp), replicating the weights across all CP ranks. Now shard across the dp-local-TP group (world / dp_size = cp_size * tp_size) -- the full TP width within one DP -- so weights are partitioned across cp*tp ranks instead of replicated. - mapping_npu: word_embed_tp_/lm_head_tp_ group_size = world/dp = cp*tp; to_json emits lm_head_tp_ (was attn_tp_). - word_embedding/lm_head/column_parallel_linear impls: use WORD_EMBED_TP / LM_HEAD_TP (ATB path) and manual dp-local-TP (TORCH path). - loaders + get_padded_vocab_size + NpuLmHeadImpl padded_vocab_size_: shard and pad by world/dp = cp*tp. - cp_size == 1 collapses to tp_size (backward compatible). - LM_HEAD_TP is only consumed by NpuLmHeadImpl; decoder last-layer AllGather uses MLP_TP (enableDpOut=false), so no double gather with gather_restore. Validated on prefill-1 (cp=32, ep=32, epd=2, MTP=3, kv_split=4): short chat and ~32k needle-in-haystack pass with no garbled output. Co-authored-by: Cursor <cursoragent@cursor.com>
Fold scattered CP prefill/ep-padding fields into npu::cp::Plan and prepare_cp_input so model/runtime share one plan accessor path. Co-authored-by: Cursor <cursoragent@cursor.com>
Consolidate CP input sharding, attention metadata, CP-EP metadata, cache-slot preparation, and output merging into NpuCpPlan with one worker-owned build entry. Replace the scattered compatibility helpers and legacy names, preserve graph tensor byte parity with focused tests, and ensure MTP target and draft leaves each build and shard exactly once. Validation: clang-format --dry-run --Werror; git diff --check; online prefill test passed (confirmed by user). Co-authored-by: Cursor <cursoragent@cursor.com>
Move WorkerImpl::prepare_npu_cp_plan into NpuCpPlan::prepare(), fed by a cached per-worker CpPlanRuntimeConfig, so the worker calls the plan in one line after the global attention-metadata consumers. shard_model_input now rewrites hidden/positions in place, merge_model_output uses the cp_group bound at prepare() time, and the worker applies attention metadata in place -- models drop modified_input_params and guard the CP path with if (cp_plan.enabled()), leaving the non-CP forward untouched. Co-authored-by: Cursor <cursoragent@cursor.com>
NpuCpPlan::to is now pulled in by ModelInputParams::to after the CP refactor, so the test needs :parallel_state to link. Co-authored-by: Cursor <cursoragent@cursor.com>
Remove historical Legacy/refactor essays and compress remaining CP comments to short one-liners so the model-side CP path stays readable. Co-authored-by: Cursor <cursoragent@cursor.com>
- reuse the original model input parameters across the MTP forward path - prepare the legacy expert array only for GLM4 MoE MTP
shifengmin
force-pushed
the
feat/npu-cp-model-closure
branch
from
July 27, 2026 09:35
5b91ef9 to
e253c01
Compare
DongheJin
previously approved these changes
Jul 27, 2026
- derive shard ownership from logical cache slots - cover multi-sequence and chunked-prefill layouts
DongheJin
approved these changes
Jul 27, 2026
Kang-Meng
approved these changes
Jul 27, 2026
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
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.
Description
Related Issues
Change Type
Pull Request Checklist
Thank you for contributing to xLLM. Before requesting review, please make sure the following items are complete.
PR Title and Commit Messages
<type>: <subject>.Pre-commit Checks
pre-commitby runningpip install pre-commitor an equivalent command.pre-commit install.pre-commit run --all-filesand fixed any reported issues.Self Review
.agents/skills/code-review/references/custom-code-style.md, especially code written or assisted by AI.mainbranch.Build and Test Coverage
python setup.py build testhas passed on a CUDA machine.python setup.py build testhas passed on an NPU machine.python setup.py build testhas passed on an MLU machine.Reviewer Notes