[CI] main2main 0703 commit: 1f486d9 - #10454
Conversation
f176660 to
2d98014
Compare
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates the project to vLLM v0.22.1 and removes legacy compatibility code that was previously required for v0.21.0. The changes involve refactoring various components to use current vLLM APIs, streamlining internal logic, and ensuring consistency across the codebase by removing version-specific branches. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the target vLLM version from v0.21.0 to v0.22.1 and cleans up the codebase by removing legacy compatibility code, conditional imports, and test skips associated with v0.21.0. The reviewer feedback is aligned with the repository style guide, providing a suggested PR Title '[CI][Misc] main2main v0.22.1' and a structured PR Summary to document these changes clearly.
| @@ -1 +1 @@ | |||
| v0.21.0 | |||
| v0.22.1 | |||
There was a problem hiding this comment.
Suggested PR Title:
[CI][Misc] main2main v0.22.1Suggested PR Summary:
### What this PR does / why we need it?
This PR updates the target vLLM version to `v0.22.1` and cleans up the codebase by removing compatibility layers, conditional imports, and test skips that were previously required for `v0.21.0`.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
CI passed with existing unit and E2E tests after removing the version-specific checks and skips.References
- The repository style guide requires providing a suggested PR Title and PR Summary in markdown code blocks following a specific format. (link)
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. |
5e6fd4d to
21cccaf
Compare
github-actions
Bot
added
the
documentation
21cccaf to
9a99536
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
1 similar comment
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
realliujiaxu
left a comment
realliujiaxu
left a comment
There was a problem hiding this comment.
Lets's merge this after v0.21.0 is released
9a99536 to
6be43f5
Compare
github-actions
Bot
added
module:core
module:quantization
merge-conflicts
and removed
merge-conflicts
labels
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
6eeeb80 to
de8fba8
Compare
de8fba8 to
25897c7
Compare
a3f68ea to
6d36e6e
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: hfadzxy <starmoon_zhang@163.com>
… no-ops vLLM 0.24.0+ (PR #44353) added start_weight_update and finish_weight_update as abstract methods on WeightTransferEngine. NPU IPC applies weights in-place via receive_weights's load_weights callback, so both are no-ops. Guard them with a v0.23.0 version check since the base class has no such methods in the older release. Signed-off-by: hfadzxy <starmoon_zhang@163.com>
…ve API torch.accelerator.get_memory_info() routes through c10's CachingDeviceAllocator and asserts the backend allocator is a DeviceAllocator. NPU's caching allocator is not registered that way, causing a crash in MemorySnapshot.__post_init__ → measure() when MemorySnapshot(device=device) is constructed on v0.24.0+. Redirect to torch.npu.mem_get_info() for NPU, matching the existing pattern of patching torch.accelerator.memory_stats, memory_reserved, reset_peak_memory_stats. Guard the patch with a v0.23.0 version check since the call path only exists on newer releases. Signed-off-by: hfadzxy <starmoon_zhang@163.com>
vLLM's InputBatch (vllm/v1/worker/gpu/input_batch.py) was refactored into a dataclass that requires two new fields: is_padding: torch.Tensor and prompt_lens: torch.Tensor | None. The v2 model runner in vllm-ascend constructs AscendInputBatch from prepare_inputs but forgot to forward those two args, leading to a TypeError: missing 2 required positional arguments: 'is_padding' and 'prompt_lens' during the first execute_model call. Match the upstream signature by slicing self.input_buffers.is_padding for is_padding and passing None for prompt_lens (R-SWA only, not yet supported on Ascend). Signed-off-by: hfadzxy <starmoon_zhang@163.com>
Upstream DeepseekV2DecoderLayer (vLLM main) now constructs the attention module with educe_results=not self.use_sequence_parallel_moe. The vllm-ascend override of DeepseekV2MLAAttention.__init__ in patch/worker/patch_deepseek_v2.py did not declare the parameter and did not forward it to the o_proj RowParallelLinear, causing TypeError: unexpected keyword argument 'reduce_results' when loading DeepSeek-V2/V3/R1 models. Add educe_results: bool = True to the init signature and pass it through to the o_proj. The RowParallelLinear parameter has existed in vLLM since at least v0.23.0, so the change is compatible across all supported versions without a version guard. Signed-off-by: hfadzxy <starmoon_zhang@163.com>
vLLM main introduced a fault-tolerance check in GPUModelRunner.__init__ that calls get_ep_all2all_manager() when data_parallel_size > 1 and is_moe. The function asserts device_communicator.all2all_manager is not None, which fails on NPU because NPUCommunicator does not register one (NPU uses mc2 / all_gather for MoE communication instead of the CUDA-style all2all manager). Add a no-op _NpuAll2AllManager to NPUCommunicator that reports support_fault_tolerance=False so the fault-tolerance path is short-circuited. Safe across vllm versions: only main reads this attribute; v0.23.0 and v0.24.0 never do. Signed-off-by: hfadzxy <starmoon_zhang@163.com>
vLLM main's FusedMoE refactor moved expert weights into a RoutedExperts submodule, so w13_weight lives at mlp.experts.routed_experts.w13_weight. vllm-ascend's AscendMoERunner and downstream model loaders still look for them at mlp.experts.w13_weight. The previous _register_routed_expert_parameter_aliases created a new nn.Parameter wrapping param.data and wrapped process_weights_after_loading to pop the aliases. This broke the gpt_oss TP=2 load because copy_/narrow on the wrapper did not consistently reach routed_experts and the pop timing clashed with the in-flight sharding. Replace the wrapper with direct Parameter sharing: register the same Parameter object on both the runner and routed_experts via _parameters dict so any in-place update propagates to both views. Drop the process_weights_after_loading wrap. Apply the alias for all MoE models in vLLM main (>=v0.24.0+ post-RoutedExperts), not just gpt_oss. v0.23.0 uses the separate AscendMoERunner in fused_moe_0_23_0.py and is unaffected. Signed-off-by: hfadzxy <starmoon_zhang@163.com>
… vllm main
Commit 77a9c5ae2 ('Weight sync refactor + move sparse nccl engine #44353')
changed the WeightTransferEngineFactory.create_engine signature from
(config, parallel_config, model) to (config, vllm_config, device, model),
and WeightTransferEngine.__init__ similarly.
Add version-guarded code paths using vllm_version_is to maintain
compatibility with both v0.23.0/v0.24.0 and the current main branch.
Changes:
- worker.py: version-guard create_engine call in load_model
- npu_ipc_engine.py: version-guard __init__ signature
- hccl_engine.py: version-guard __init__ signature + add
start_weight_update/finish_weight_update for main
Signed-off-by: hfadzxy <starmoon_zhang@163.com>
…end MC2 dispatch
vllm main (post PR #41184 MoE Refactor) introduced an all-gather of
[hidden_states, residual] inside DeepseekV2Model.forward when
use_sequence_parallel_moe is False. This all-gather conflicts with
vllm-ascend's MC2 dispatch, which internally handles TP partitioning
via mc2_mask slicing in prepare_finalize.py.
When the model-level all-gather runs, it expands TP-partitioned
hidden_states to full size, but mc2_mask remains sized for the
partition, causing a shape mismatch in npu_moe_distribute_dispatch_v2:
xActiveMask dim0 = 2 is not equal to x dim0 = 4
Fix: monkey-patch DeepseekV2Model.forward to remove the all-gather
path entirely. The MC2 dispatch handles TP partitioning internally,
so the model-level gather is both unnecessary and incompatible.
Also fixes a secondary crash: when residual is None (first layer,
first PP stage), the upstream code calls torch.cat([h, None])
which raises TypeError during torch.compile tracing.
The patch is guarded with vllm_version_is('0.23.0') since v0.23.0
does not have the problematic all-gather code.
Signed-off-by: hfadzxy <starmoon_zhang@163.com>
Upstream PR #44589 (Remove unnecessary load_weights methods) changed weight loading timing, causing all MoE weights to reside on NPU simultaneously before process_weights_after_loading runs. For large MoE models (e.g. Qwen3-Next-80B-A3B with TP=4), the .contiguous() call after .transpose() doubles peak memory, triggering OOM. Remove .contiguous() on non-v0.23.0 versions: .transpose(1,2) already produces the correct logical layout as a zero-copy view. The subsequent npu_format_cast / maybe_trans_nz handles the non-contiguous tensor and produces contiguous NZ-format storage, avoiding the transient double allocation. Signed-off-by: hfadzxy <starmoon_zhang@163.com>
| w2_data = maybe_trans_nz(w2_data) | ||
| layer.w2_weight = torch.nn.Parameter(w2_data, requires_grad=False) | ||
| if not vllm_version_is("0.23.0"): | ||
| w13_data = self._maybe_pad_weight(layer.w13_weight.data).transpose(1, 2) |
There was a problem hiding this comment.
why we need remove contiguous here?
There was a problem hiding this comment.
- Peak memory reduction. After upstream PR #44589 changed the weight loading timing, all MoE weights reside on NPU simultaneously during process_weights_after_loading. .transpose(1,2) followed by .contiguous() materializes a full copy, doubling the memory footprint — causing OOM on large MoE models like Qwen3-Next-80B-A3B (TP=4).
- Redundant operation. .transpose(1,2) is already a zero-copy view with correct logical layout. The subsequent maybe_trans_nz() handles non-contiguous inputs and outputs contiguous NZ storage, so the intermediate .contiguous() is unnecessary.
| w13_data = self._maybe_pad_weight(layer.w13_weight.data).transpose(1, 2).contiguous() | ||
| layer.w13_weight = torch.nn.Parameter(w13_data, requires_grad=False) | ||
| if not vllm_version_is("0.23.0"): | ||
| w13_data = self._maybe_pad_weight(layer.w13_weight.data).transpose(1, 2) |
There was a problem hiding this comment.
ditto, why remove contiguous here?
| def _register_routed_expert_parameter_aliases(self) -> None: | ||
| alias_names = [] | ||
| for name, param in self.routed_experts.named_parameters(recurse=False): | ||
| alias_param = torch.nn.Parameter(param.data, requires_grad=param.requires_grad) | ||
| alias_param.__dict__.update(param.__dict__) | ||
| self.register_parameter(name, alias_param) | ||
| alias_names.append(name) | ||
|
|
||
| original_process_weights = self._quant_method.process_weights_after_loading | ||
|
|
||
| @wraps(original_process_weights) | ||
| def wrapped_process_weights(layer, *args, **kwargs): | ||
| for name in alias_names: | ||
| self._parameters.pop(name, None) | ||
| return original_process_weights(layer, *args, **kwargs) | ||
|
|
||
| self._quant_method.process_weights_after_loading = wrapped_process_weights # type: ignore[method-assign] | ||
| object.__setattr__(self, name, param) | ||
|
|
||
| def _needs_routed_expert_parameter_aliases(self) -> bool: | ||
| vllm_config = get_current_vllm_config() | ||
| hf_config = getattr(vllm_config.model_config, "hf_config", None) | ||
| return getattr(hf_config, "model_type", None) == "gpt_oss" | ||
| return True |
There was a problem hiding this comment.
let's remove these 2 functions if we don't need to alia parameters
| islice(self.layers, self.start_layer, self.end_layer), | ||
| start=self.start_layer, | ||
| ): | ||
| if idx in self.aux_hidden_state_layers: |
There was a problem hiding this comment.
I think we could keep the same implementation with vLLM, cause the all-gather won't be triggered if we don't enable sp with the same way of vLLM. @linfeng-yuan please also take a look.
There was a problem hiding this comment.
The patch is necessary, confirmed by experiment: removing it causes test_deepseek_v2_lite_fc1_tp2 to crash immediately (torch.cat([hidden_states, None]) → TypeError at deepseek_v2.py:1460).
vLLM SP and Ascend MC2 are mutually incompatible communication paths. vLLM SP requires model-level all-gather to expand hidden_states to full size; MC2 requires hidden_states to stay partition-shaped, with mc2_mask handling slicing internally. Enabling both causes xActiveMask dim0 != x dim0. This is also why platform.py:648-650 explicitly disables use_sequence_parallel_moe.
The per-layer all-gather in upstream is still needed for GPU non-SP paths, so it's not suitable for upstream removal. However, the missing residual is None guard in torch.cat is a cross-platform defect worth upstreaming. The current patch remains the cleanest fix for Ascend.
4 participants
What this PR does / why we need it?
vllm_ascend/distributed/weight_transfer/npu_ipc_engine.py
vllm_ascend/distributed/weight_transfer/hccl_engine.py
vllm_ascend/patch/platform/patch_torch_accelerator.py
vllm_ascend/patch/worker/patch_qwen3_dflash.py
vllm_ascend/worker/v2/model_runner.py
vllm_ascend/patch/worker/patch_v2/patch_input_batch.py
vllm_ascend/patch/worker/patch_deepseek_v2.py
vllm_ascend/distributed/device_communicators/npu_communicator.py
vllm_ascend/ops/fused_moe/fused_moe.py
vllm_ascend/worker/worker.py
vllm_ascend/distributed/weight_transfer/npu_ipc_engine.py
vllm_ascend/distributed/weight_transfer/hccl_engine.py
vllm_ascend/patch/platform/patch_weight_transfer_engine.py
vllm_ascend/patch/worker/patch_deepseek_v2.py
vllm_ascend/ops/fused_moe/fused_moe.py
load_weightsmethods vllm#44589)Does this PR introduce any user-facing change?
How was this patch tested?