[BugFix] fix dsv4 piecewise scenario - #10003
Conversation
Signed-off-by: zouyida2052 <zouyida2002@gmail.com>
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 refactors the configuration logic for Ascend platforms to ensure that the breakable cudagraph feature is correctly disabled. By moving the environment variable override to the module initialization, the change prevents potential conflicts introduced by recent upstream changes in vLLM regarding model compilation support. 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. 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
|
There was a problem hiding this comment.
Code Review
This pull request moves the logic that disables breakable CUDA graphs on Ascend from the apply_config_platform_defaults method to the module level of vllm_ascend/platform.py. The reviewer suggests also setting the environment variable os.environ["VLLM_USE_BREAKABLE_CUDAGRAPH"] = "False" to ensure that direct environment variable lookups respect this override, and provides a suggested PR title and summary in accordance with the repository's style guide.
| # Since vllm-project/vllm#43746, DeepSeek V4 model classes no longer | ||
| # carry @support_torch_compile. This makes vLLM auto-enable the breakable | ||
| # cudagraph PIECEWISE path, which is not supported on Ascend yet. | ||
| envs_vllm.VLLM_USE_BREAKABLE_CUDAGRAPH = False |
There was a problem hiding this comment.
To ensure that any direct environment variable lookups (e.g., via os.getenv or os.environ.get) also respect this override, and to handle cases where other modules might import or check the environment variable directly, we should set both os.environ["VLLM_USE_BREAKABLE_CUDAGRAPH"] = "False" and envs_vllm.VLLM_USE_BREAKABLE_CUDAGRAPH = False.
Suggested PR Title:
[Ops][BugFix] fix dsv4 piecewise scenarioSuggested PR Summary:
### What this PR does / why we need it?
This PR force disables the breakable CUDA graph PIECEWISE path on Ascend at the module level of `vllm_ascend/platform.py`. This is necessary because DeepSeek V4 model classes no longer carry `@support_torch_compile`, causing vLLM to auto-enable the breakable CUDA graph PIECEWISE path, which is not yet supported on Ascend.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
CI/CD testing.os.environ["VLLM_USE_BREAKABLE_CUDAGRAPH"] = "False"
envs_vllm.VLLM_USE_BREAKABLE_CUDAGRAPH = FalseReferences
- Pull Request Summary Style Guide: Output the generated title and summary in markdown code blocks. (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. |
### What this PR does / why we need it? In [PR#43746](vllm-project/vllm#43746 VLLM_USE_BREAKABLE_CUDAGRAPH is detacted as True, compilation_config.mode would be None even if we set it, thus server runs in eager mode. ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@9090368 --------- Signed-off-by: zouyida2052 <zouyida2002@gmail.com>
… into main-dsv4 * 'main-dsv4' of https://github.com/chen-commits/vllm-ascend: (35 commits) 测试qwen3.5-122B [BugFix][UT] Fix platform eager mode log assertion (vllm-project#10075) [Misc] Use logger.info_once for enable_npugraph_ex log (vllm-project#9699) [CI] delete DeepSeek-R1-W8A8.yaml update DeepSeek-R1-0528-W8A8-EPLB.yaml (vllm-project#10055) [CI] fix ci error when rebase on latest main (vllm-project#10057) [CI] Upgrade the image version of pr_e2e_comment (vllm-project#10052) [BugFix] Include zero cached prompt token details (vllm-project#9911) [Doc][Misc] Update release notes known issues (vllm-project#10039) [Doc]Update MiniMax-M2.5.md change A2 to single node (vllm-project#9481) [Doc]Add InternVL3.5 documentation (vllm-project#9980) [BugFix]Fix the question that cant find num_kv_head from some model config (vllm-project#10008) [Ascend950][BugFix] Fix MoonViT3dPretrainedModel.to overriding quantized ViT weight dtype (vllm-project#9929) [CI][Test] Optimize selective test routing and CI test stability (vllm-project#9994) [BugFix] fix dsv4 piecewise scenario (vllm-project#10003) [Test][CI] final part for selected test (vllm-project#10017) [Feature] Enable prefix caching with PCP/DCP (vllm-project#9638) [Doc]Update GLM-5 documentation and support matrix (vllm-project#9889) [CI] add MiniMax A2 acc case (vllm-project#9855) [Bugfix] [kimi] upgrade transformers from 5.5.3 to 5.5.4 (vllm-project#9996) [Doc][Model] Add DeepSeek-V4 Flash and Pro documentation (vllm-project#9995) ...
### What this PR does / why we need it? In [PR#43746](vllm-project/vllm#43746 VLLM_USE_BREAKABLE_CUDAGRAPH is detacted as True, compilation_config.mode would be None even if we set it, thus server runs in eager mode. ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@9090368 --------- Signed-off-by: zouyida2052 <zouyida2002@gmail.com>
### What this PR does / why we need it? In [PR#43746](vllm-project/vllm#43746 VLLM_USE_BREAKABLE_CUDAGRAPH is detacted as True, compilation_config.mode would be None even if we set it, thus server runs in eager mode. ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@9090368 --------- Signed-off-by: zouyida2052 <zouyida2002@gmail.com>
### What this PR does / why we need it? In [PR#43746](vllm-project/vllm#43746 VLLM_USE_BREAKABLE_CUDAGRAPH is detacted as True, compilation_config.mode would be None even if we set it, thus server runs in eager mode. ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@9090368 --------- Signed-off-by: zouyida2052 <zouyida2002@gmail.com>
### What this PR does / why we need it? In [PR#43746](vllm-project/vllm#43746 VLLM_USE_BREAKABLE_CUDAGRAPH is detacted as True, compilation_config.mode would be None even if we set it, thus server runs in eager mode. ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@9090368 --------- Signed-off-by: zouyida2052 <zouyida2002@gmail.com>
### What this PR does / why we need it? In [PR#43746](vllm-project/vllm#43746 VLLM_USE_BREAKABLE_CUDAGRAPH is detacted as True, compilation_config.mode would be None even if we set it, thus server runs in eager mode. ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@9090368 --------- Signed-off-by: zouyida2052 <zouyida2002@gmail.com>
### What this PR does / why we need it? In [PR#43746](vllm-project/vllm#43746 VLLM_USE_BREAKABLE_CUDAGRAPH is detacted as True, compilation_config.mode would be None even if we set it, thus server runs in eager mode. ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@9090368 --------- Signed-off-by: zouyida2052 <zouyida2002@gmail.com>
What this PR does / why we need it?
In PR#43746,while VLLM_USE_BREAKABLE_CUDAGRAPH is detacted as True, compilation_config.mode would be None even if we set it, thus server runs in eager mode.
Does this PR introduce any user-facing change?
no
How was this patch tested?