[v0.26.0] Upgrade GPU and NPU backends - #186
Conversation
Signed-off-by: zzh <jiaranran2@gmail.com>
[v0.26.0][GPU] Integrate DeepSeek remote-experts refactor
Signed-off-by: jiangkuaixue123 <jiangxiaozhou111@163.com>
feat(npu): support vLLM 0.26
Signed-off-by: jiangkuaixue123 <jiangxiaozhou111@163.com>
refactor(npu): remove PCP support from model runner v1
Signed-off-by: jiangkuaixue123 <jiangxiaozhou111@163.com>
fix(npu): restore DBO compatibility for vLLM 0.26
Signed-off-by: jiangkuaixue123 <jiangxiaozhou111@163.com>
Signed-off-by: jiangkuaixue123 <jiangxiaozhou111@163.com>
jiangkuaixue123
left a comment
There was a problem hiding this comment.
Scan:
| Category | Result |
|---|---|
| Tests / verification | 1 finding(s) below |
| Security | no finding reported |
| Docs / comments | 3 finding(s) below |
| Behavior / compatibility | 1 finding(s) below |
| Correctness | no finding reported |
Validated:
- [sweep] config_validation.py:56-91 — all branches in
create_engine_configcan occur; worker_cls_was_auto captured before backend relaxation; Ascend patch applied before upstream config creation; backend restores are ordered correctly - [sweep] config_validation.py:99-116 —
__post_init__now only handles backend relaxation; worker selection moved tocreate_engine_config; no merge damage - [sweep] config_validation.py:148-165 —
_should_relax_engine_args_backendreplaced getattr with direct attribute access;nixl_epadded to exclusion set;enable_dbo/ubatch_sizeguard matches EngineArgs-level semantics - [sweep] config_validation.py:168-181 —
_should_relax_vllm_config_backendusesuse_ubatchingvs EngineArgs-levelenable_dbo/ubatch_size— intentional divergence between first-validation and re-validation paths - [sweep] ascend_platform.py:29-71 —
apply_afd_ascend_dbo_config_patchnow patchescheck_and_update_config(not_fix_incompatible_config) and returns bool; snapshot/restore wraps whole method - [sweep] ascend_platform.py:74-100 —
_AFDDBOConfigSnapshotno longer capturesuse_ubatching;_restore_afd_dbo_configguard changed from 3-condition OR to 2-condition AND; see finding below - [validated] ascend_platform.py:99 — guard
not saved.enable_dbo and saved.ubatch_size == 0is equivalent to old guard minususe_ubatching; early return when nothing to restore is correct - [sweep] force_load_balance.py:207-243 —
__init__onAscendW8A8DynamicFusedMoEMethodcaptures AFD config only; does NOT callsuper().__init__()and does NOT setself.quant_type; see finding below
Reviewed 12 candidates. Dropped 4 (i0 misread the patched init, i1/i9 speculation about use_ubatching without evidence, i3 misinterpreted the guard logic). Kept 8: one minor confirmation request for the unconditional output queue task, one nit about the engine_id comment, one stale artifact, two documentation updates, two verification asks (add a comment about router_logits on NPU and a test for closed-connector error path), and one test-assertion suggestion.
Verdict: COMMENT
Findings without a valid current diff anchor
These findings are preserved in the review body because their file/line could not be mapped to the fetched PR diff.
vllm_afd_plugin.egg-info/PKG-INFO:46[minor] — vllm_afd_plugin.egg-info/PKG-INFO still references vLLM 0.19.1 (grep returned 6 matches). Either regenerate the metadata during the build or add vllm_afd_plugin.egg-info/ to .gitignore to prevent stale artifacts. Evidence: grep for '0.19.1' in vllm_afd_plugin.egg-info/PKG-INFO returned 6 matches all with old version references. The README.md and other source files in the diff correctly updated their references.afd_plugin/v1/worker/npu/ffn_model_runner.py:246[minor] — The NPU FFN runner (_ffn_forward at lines 235-265) receives AFDA2FTransferPayload but never accesses payload.router_logits (unlike the GPU runner). Add a comment or assertion that the synchronous CAMP2P path does not use router_logits (GPU remote-experts only). Evidence: Read afd_plugin/v1/worker/npu/ffn_model_runner.py lines 235-265 (the _ffn_forward method). The payload destructuring at line 253-255 assigns hidden_states, context, metadata, states but never reads router_logits. Compare with GPU FFN runner at ffn_model_runner.py line 218-222 which conditionally reads payload.router_logits.
| # wakeup while preserving normal routing. | ||
| if not self.engines_running and not is_afd_async_dp(self.vllm_config): | ||
| req_msg = core_client_module.msgspec.msgpack.encode( | ||
| ("FIRST_REQ", chosen_engine), |
There was a problem hiding this comment.
[minor] The unified add_request_async now calls self._ensure_output_queue_task() unconditionally (line 358), while the previous AFD async-DP path (deleted lines ~329-341) explicitly omitted it. Confirm whether this behavioral change is intentional or if the AFD path should still skip the output queue task.
Evidence: Read async_dp_engine.py: the old code had two separate branches; the AFD branch (after # PATCH START) omitted _ensure_output_queue_task(). The new unified code calls it unconditionally at line 358.
| # Patch reason: native DeepseekV2MoE constructs local routed/shared experts. | ||
| # Patch functionality: preserve the native MoE forward contract while | ||
| # constructing only the gate owned by Attention and a parameter-free proxy. | ||
| # Signature: AFD-owned; adds layer_idx and gate_placement and omits |
There was a problem hiding this comment.
[minor] The patch comment at line 244 says 'adds layer_idx and gate_placement' but the actual parameter is named compute_gate_on_attention (line 251). Update the comment to match the actual signature.
Evidence: File afd_plugin/model_executor/models/deepseek_v2.py: patch comment at line 244 reads # Signature: AFD-owned; adds layer_idx and gate_placement and omits while the init signature at line 251 defines compute_gate_on_attention: bool. Grep for gate_placement across the repo confirms this string appears nowhere else — it was renamed but the comment was not updated.
|
|
||
| vLLM-Ascend's platform compatibility pass disables DBO/ubatching fields for | ||
| ordinary NPU runs. AFD owns its NPU ubatching path, so this patch snapshots | ||
| those fields for AFD-enabled configs, lets upstream normalization run, then |
There was a problem hiding this comment.
[minor] The docstring for apply_afd_ascend_dbo_config_patch (lines 27-37) only mentions DBO/ubatching fields, but the snapshot now includes all2all_backend. Update the docstring to also mention that all2all_backend is preserved, consistent with the inline comment at line 56.
Evidence: File afd_plugin/compat/patches/npu/ascend_platform.py: docstring at lines 27-37 describes the snapshot/restore flow but only mentions DBO/ubatching fields. _AFDDBOConfigSnapshot at line 24 includes all2all_backend: str. _restore_afd_dbo_config at line 105 restores parallel_config.all2all_backend = saved.all2all_backend. The inline patch comment at lines 55-60 already correctly mentions both DBO fields and the temporary ubatching backend.
|
please update the copilot knowledge/rules as well after this upgrade |
|
please create a release for v0.19.1 before this PR merged. the whl is enough |
hsliuustc0106
left a comment
There was a problem hiding this comment.
Three inline findings from the vLLM 0.26 compatibility review.
| ): | ||
| router_logits = payload.router_logits | ||
| assert router_logits is not None | ||
| rank_ffn_output = self.model.compute_experts_output( |
There was a problem hiding this comment.
[P1] Aggregate FFN DP sizes before the external-router MoE call
The newly supported CUDA external-router branch enters native MoE after P2P fan-in, but forward_context.dp_metadata still carries raw per-Attention counts. In documented-valid 4A2F, counts [3, 5, 7, 11] make F0 receive 8 concatenated tokens while vLLM’s AgRs path reads local size 3 and asserts sizes[rank] == hidden_states.shape[0]; the FFN rank aborts while Attention waits for its response. The raw-metadata defect predates this PR for gate-on-FFN, but this PR newly routes supported CUDA compute_gate_on_attention=true through it. Please derive FFN-side subgroup totals ([8, 18] here) after TP expansion while retaining raw counts for P2P wire allocation, or reject A != F. Add a native AgRs 4A2F test for both gate placements.
|
|
||
| # Patch reason: native SM partitioning conflicts with AFD connector work. | ||
| # Patch functionality: disable native SM partitioning only for active AFD. | ||
| # Signature: matches upstream; no added parameters. |
There was a problem hiding this comment.
[P2] Keep the declared patch signature exact
This new comment says the signature matches pinned vLLM, but upstream UBatchWrapper._create_sm_control_context(vllm_config: VllmConfig) has no return annotation while this override declares -> AbstractContextManager[None]. AGENTS.md explicitly requires patched signatures—including return types—to match exactly. Please remove the annotation or document it as an intentional signature delta instead of claiming an exact match.
| | [`engine_core.py`](../../../afd_plugin/compat/patches/engine_core.py): `EngineCore.__init__`, `_initialize_kv_caches`, `shutdown`; `EngineCoreProc.run_busy_loop`; `DPEngineCoreProc.run_busy_loop` | AFD FFN becomes a connector daemon: construct executor, skip scheduler/KV setup, return an empty KV-shaped result on late paths, start/monitor/stop the FFN worker loop, and use FFN-safe shutdown. Non-FFN branches copy pinned upstream behavior. | Imported by `register_afd`; **no patch-local version guard and no saved-original sentinel**. Direct class assignment means the package pin and review discipline are the compatibility guard. | [`test_engine_core.py`](../../../tests/unit/compat/patches/test_engine_core.py) covers FFN initialization, non-FFN behavior, and daemon start/stop; role runtime tests cover error propagation. | Remove when vLLM offers a headless connector-daemon engine lifecycle or an executor mode that does not require scheduler/KV ownership. | | ||
| | [`npu/ascend_platform.py`](../../../afd_plugin/compat/patches/npu/ascend_platform.py): `NPUPlatform._fix_incompatible_config` | Snapshots AFD DBO state, runs upstream normalization, and restores configured `enable_dbo`/`ubatch_size` when AFD needs them; non-AFD behavior is unchanged. | Called through `apply_afd_ascend_patches_if_needed`; no version guard. Saves the original on the class and uses both a class sentinel and runtime-facade sentinel. Missing vLLM-Ascend is a no-op. | [`test_runtime.py`](../../../tests/unit/compat/test_runtime.py) and [`test_npu_runtime.py`](../../../tests/unit/v1/worker/test_npu_runtime.py). | Remove when vLLM-Ascend recognizes plugin-owned DBO workers or no longer clears these fields. | | ||
| | [`npu/ascend_platform.py`](../../../afd_plugin/compat/patches/npu/ascend_platform.py): `NPUPlatform.check_and_update_config` | Snapshots AFD DBO state, runs upstream normalization, and restores configured `enable_dbo`, `ubatch_size`, and `all2all_backend` in `finally`; non-AFD behavior is unchanged. | Called through `apply_afd_ascend_patches_if_needed`; no version guard. Saves the original on the class and uses a class sentinel. The runtime facade caches success only after the wrapper is installed, so an early missing vLLM-Ascend import remains retryable. | [`test_runtime.py`](../../../tests/unit/compat/test_runtime.py) and [`test_npu_runtime.py`](../../../tests/unit/v1/worker/test_npu_runtime.py). | Remove when vLLM-Ascend recognizes plugin-owned DBO workers or no longer clears these fields. | | ||
| | [`npu/force_load_balance.py`](../../../afd_plugin/compat/patches/npu/force_load_balance.py): `AscendFusedMoE.__init__`, `AscendW8A8DynamicFusedMoEMethod.apply` | Adds AFD profiling configuration and replaces routed expert IDs with a deterministic balanced buffer only when the layer-owned switch is enabled; normal model-selected routing remains unchanged. This switch changes outputs and is not a correctness feature. | Imported only when vLLM-Ascend is discoverable; **no patch-local version guard or explicit reload sentinel**. Functions copy the current upstream bodies with marked AFD deltas. | [`test_force_load_balance.py`](../../../tests/unit/compat/patches/test_force_load_balance.py) covers buffer bounds, determinism, growth, override, and pass-through. | Upstream a deterministic expert-routing profiling hook in vLLM-Ascend, then delete both copied functions. | |
There was a problem hiding this comment.
[P2] Update the force-load-balance patch inventory
This row still says the plugin patches AscendFusedMoE.__init__ and uses a layer-owned switch. The refreshed implementation now globally assigns both AscendW8A8DynamicFusedMoEMethod.__init__ and .apply (force_load_balance.py:442-443), with method-owned state; there is no remaining AscendFusedMoE assignment. Please update the symbol inventory and ownership wording so future vLLM-Ascend upgrade/removal audits inspect the actual patch.
ok |
ok |
Purpose
Upgrade the AFD GPU and NPU backends together from the vLLM 0.19.1 contract to vLLM 0.26.0.
This PR promotes the completed
update/v0.26.0integration branch tomainas one combined change. It brings together the GPU remote-experts refactor from #182 and the NPU runtime, PCP-removal, and DBO compatibility work from #183, #184, and #185.Issue
Scope
Implementation Notes
The GPU path keeps native
Model.forward,DecoderLayer.forward, andDeepseekV2MoE.forward, and installs a parameter-free Attention experts proxy at the remote-experts boundary. Routed and shared experts remain owned by the FFN model runners, and the existing connector protocol is reused.The NPU path rebases plugin-owned runners and isolated compatibility patches onto the v0.26 APIs. It removes stale PCP fields and hooks, refreshes the DeepSeek
FusedMoEfactory after plugin initialization, captures force-load-balance configuration during method construction, and preserves AFD DBO settings across Ascend platform validation.Test Plan
Test Result
git diff --check, and the pinned vLLM-Ascend installation link check passed.git diff --check upstream/main...upstream/update/v0.26.0passed.Docs Impact
80d8c194f.Essential PR Checklist