[3/N][CP] Implement zigzag CP strategy - #28421
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements the Zigzag Context Parallel (CP-v2) strategy, replacing previous placeholders with concrete implementations for metadata building, hidden state and KV cache sharding/gathering, and attention dispatch. It also integrates these changes into the model runner and adds corresponding unit and integration tests. The review feedback highlights two critical issues: first, a device fallback mechanism in zigzag.py that may fail on non-CUDA platforms (such as NPU or MUSA) and should instead query the global server arguments; second, a bug in _forward_extend_cp_v2 where the auxiliary hidden states tuple is not correctly returned on non-last pipeline parallel ranks when capture_aux_hidden_states is enabled.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/rerun-test test/registered/cp/test_cp_strategy_unit.py test/registered/cp/test_gqa_preill_cp.py |
|
Results for 🚀 🚀 |
7971196 to
3bf9568
Compare
|
/rerun-test test/registered/cp/test_cp_strategy_unit.py test/registered/cp/test_gqa_preill_cp.py |
|
Results for 🚀 🚀 |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
3bf9568 to
f6d6201
Compare
|
/rerun-test test/registered/cp/test_cp_strategy_unit.py test/registered/cp/test_gqa_preill_cp.py |
|
Results for 🚀 🚀 |
|
/rerun-test test/registered/cp/test_cp_strategy_unit.py test/registered/cp/test_gqa_preill_cp.py test/registered/cp/test_gqa_prefill_cp_legacy.py |
|
Results for 🚀 🚀 |
706f221 to
0030086
Compare
|
/rerun-test test/registered/cp/test_cp_strategy_unit.py test/registered/cp/test_gqa_preill_cp.py test/registered/cp/test_gqa_prefill_cp_legacy.py |
|
Results for 🚀 🚀 |
|
/rerun-test test/registered/unit/server_args/test_server_args.py |
|
Results for 🚀 |
|
/rerun-test test/registered/cp/test_deepseek_v32_cp_single_node.py test/registered/cp/test_deepseek_v3_cp_single_node.py test/registered/cp/test_deepseek_v4_flash_fp4_b200_cp.py |
|
Results for 🚀 🚀 |
|
/rerun-test registered/cp/test_gqa_preill_cp.py |
|
Results for 🚀 |
kpham-sgl
left a comment
There was a problem hiding this comment.
Mostly self reminders, thanks for the refactor
| ) -> Any: | ||
| raise NotImplementedError( | ||
| "Zigzag attention dispatch will land in a follow-up PR" | ||
| assert ( |
There was a problem hiding this comment.
Need similar logic here, as currently zigzag's build_metadata does not modify / is not aware of seq_lens_k
Without this padding, for CP=4 in GQA you may run into IMA
There was a problem hiding this comment.
Be careful with the padding happens inside
| self.assertIsNotNone(get_cp_strategy()) | ||
|
|
||
|
|
||
| class TestCPZigzagStrategy(CustomTestCase): |
There was a problem hiding this comment.
May need to fuse / add attention computation unit tests (like the new attention test kit)
Route MLA prefill context parallelism (DeepSeek V3/R1) through the layers/cp ZigzagCPStrategy abstraction, mirroring the GQA path from #28421. DeepSeek V3 is added to CP_V2_DEFAULT_MODEL_CLASSES so CP-v2 is the default for it (as with Qwen3Moe); the legacy cp_utils v1 path stays intact and reachable via SGLANG_ENABLE_CP_V2=0. Under v2 the eager runner owns the entry shard (cp_split_before_forward) and exit gather (cp_gather_after_forward) and drives DeepseekV2Model.forward directly, so: - deepseek_v2.py / deepseek_nextn.py: gate the in-model CP entry split and exit gather to the v1-only path (`and not is_cp_v2_active`). - flashattention_backend.py: MLA CP attention dispatches through cp_strategy.run_attention under v2, else the v1 cp_attn_forward_extend. The MLA latent all-gather (rebuild_cp_kv_cache) stays upstream in forward_absorb_prepare for both paths; zigzag metadata is field-compatible with the v1 ContextParallelMetadata, so no KV-write change is needed. Pure-MLA arch only: DSA archs (DeepseekV32/DeepseekV4) are excluded (CP-v2 does not yet cover DSA), and Kimi K2.5 needs separate wiring for its .language_model wrapper. The existing test_deepseek_v3_cp_single_node.py (tp8 dp2 attn-cp4) now exercises CP-v2 via the default-class enablement. Part of the CP refactor roadmap (#27252) / context parallelism roadmap (#21788). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Decision A — move the MLA latent all-gather into the CP strategy for v2: - forward_mla.py: skip the upstream rebuild_cp_kv_cache under CP-v2 (k_nope/k_pe stay rank-local through forward_absorb_core). - flashattention_backend.py: the MLA KV write dispatches to cp_strategy.materialize_full_mla_kv under v2 (gather rank-local latent to full, then set_mla_kv_buffer); v1 keeps the upstream-rebuilt full write. - zigzag.py: add materialize_full_mla_kv (fuse compressed-KV + rope, one all-gather via gather_kv_cache, split back for the paged write). Kimi K2.5 wiring: - eager_runner.py: drive resolve_cp_forward_model(model) instead of the raw model, so multimodal wrappers expose their inner CausalLM for the CP-v2 path. - kimi_k25.py: KimiK25ForConditionalGeneration.get_cp_model() returns language_model. - cp/utils.py: add resolve_cp_forward_model + KimiK25ForConditionalGeneration to CP_V2_DEFAULT_MODEL_CLASSES. The seq_lens_k padding flagged on #28421 is already handled (zigzag build_metadata bakes pad_len; the FA backend widens page_table by pad_delta); Decision A's gather preserves the same padded length. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
SGLANG_ENABLE_CP_V2=1.cp_utils.py.--attn-cp-size 4 --ep 4 --moe-a2a-backend deepep.Notes
SGLANG_ENABLE_CP_V2=1is set.CI States
Latest PR Test (Base): 🚫 Run #27791684107
Latest PR Test (Extra): ❌ Run #27791684005