fix(npu): adapt colocate patch for vllm-ascend and vllm version upgrade#318
fix(npu): adapt colocate patch for vllm-ascend and vllm version upgrade#318miracle0517 wants to merge 1 commit into
Conversation
Signed-off-by: wuxiang <498160096@qq.com>
Documentation build overview
42 files changed ·
|
![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 configures line endings for patch files, cleans up commented-out lines in the vllm-ascend.patch file, adds a patch to disable flash attention rotary embedding on NPU platforms, and sets TRANSFORMERS_VERBOSITY=error in the Qwen3 NPU execution script. The feedback highlights a potential risk in using hasattr(current_platform, "is_npu") to check for NPU platforms, as it could cause performance regressions on other platforms if is_npu is added to the base platform class in the future. A safer alternative using getattr with a default fallback callable is suggested.
|
|
||
| self.apply_rotary_emb_flash_attn = None | ||
| - if not current_platform.is_cpu() and find_spec("flash_attn") is not None: | ||
| + if not current_platform.is_cpu() and find_spec("flash_attn") is not None and not hasattr(current_platform, "is_npu"): |
There was a problem hiding this comment.
Using not hasattr(current_platform, "is_npu") to check if the platform is not an NPU is risky and not future-proof. If a future version of vLLM adds the is_npu method to the base Platform class (returning False by default, similar to is_xpu or is_hpu), then hasattr(current_platform, "is_npu") will evaluate to True on all platforms, including CUDA. This would cause the condition to evaluate to False on CUDA, silently disabling the flash attention rotary embedding and leading to a significant performance regression.\n\nA safer and more robust approach is to check if the attribute exists and call it, or use getattr with a default callable:\nnot getattr(current_platform, "is_npu", lambda: False)()
+ if not current_platform.is_cpu() and find_spec("flash_attn") is not None and not getattr(current_platform, "is_npu", lambda: False)():
miracle0517
changed the title
|
Version adaptation looks good. Going forward (not blocking): could the colocate weight sync migrate to fully reuse the native NPUIPCWeightTransferEngine instead of maintaining the custom _patch_npu_colocate_worker / update_weights_chunk path? Would save carrying this patch long-term. Context: #285 (comment) — worth tracking as a follow-up on your side? cc @CalvinXKY |
3 participants
Summary
Test