[BugFix] Fix ascend config check #1092
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wangxiyuan
force-pushed
the
fix_ascend_config
branch
from
372e466 to
b9a4e95
Compare
github-actions
bot
added
the
documentation
Yikun
approved these changes
Jun 5, 2025
wangxiyuan
force-pushed
the
fix_ascend_config
branch
3 times, most recently
from
324099c to
d1439b7
Compare
NeverRaR
reviewed
Jun 5, 2025
vllm_ascend/ascend_config.py
Outdated
| @@ -126,7 +129,7 @@ def check_ascend_config(vllm_config, enforce_eager): | |||
| "Torchair graph mode only works with deepseek model.") | |||
|
|
|||
| # for V1 Engine, aclgraph doesn't work with deepseek model and only qwen model is well tested. | |||
| if envs.VLLM_USE_V1 and vllm_config.model_config is not None and not enforce_eager: | |||
| if envs.VLLM_USE_V1 and vllm_config.model_config is not None and not enforce_eager and not ascend_config.torchair_graph_config.enabled: | |||
There was a problem hiding this comment.
this check should be moved to platform.py:check_and_update_config
NeverRaR
suggested changes
Jun 5, 2025
vllm_ascend/ascend_config.py
Outdated
| @@ -126,7 +129,7 @@ def check_ascend_config(vllm_config, enforce_eager): | |||
| "Torchair graph mode only works with deepseek model.") | |||
There was a problem hiding this comment.
should be:
if ascend_config.torchair_graph_config.enabled:
if envs.VLLM_MLA_DISABLE:
logger.warning(
"Torchair graph mode is still experimental and not supported for V1 without mla currently, "
"it has been disabled automatically.")
ascend_config.torchair_graph_config.enabled = False
elif vllm_config.model_config:
model_type = vllm_config.model_config.hf_config.model_type
if "deepseek" not in model_type:
raise NotImplementedError(
"Torchair graph mode only works with deepseek model.")
ascend_config. ascend_scheduler_config.enabled = False -> ascend_config.torchair_graph_config.enabled = False
if vllm_config.model_config: -> elif vllm_config.model_config:
wangxiyuan
force-pushed
the
fix_ascend_config
branch
from
d1439b7 to
3eefdc9
Compare
|
@NeverRaR Thanks for your review. I've updated the check logic to make it more clear. Please take a look at again. |
Yikun
reviewed
Jun 6, 2025
| @@ -29,6 +29,7 @@ The following table lists the additional configuration options available in vLLM | |||
| | `torchair_graph_config` | dict | `{}` | The config options for torchair graph mode | | |||
| | `ascend_scheduler_config` | dict | `{}` | The config options for ascend scheduler | | |||
| | `expert_tensor_parallel_size` | str | `1` | Expert tensor parallel size the model to use. | | |||
| | `refresh` | bool | `false` | Whether to refresh global ascend config content. This value is usually used by rlhf case. | | |||
There was a problem hiding this comment.
QQ: Does vLLM has similar config?
There was a problem hiding this comment.
Not, this value is only used for rlhf, for verl or someother framework, the case is:
- verl load and update vllm config
- verl start LLM with additional config in external_executor mode
in the fisrt step, the ascend config has been initialized, then in the second step, the additional config will be skipped.
To solve the problem, we should let verl pass refresh, the we can regenerate the config
NeverRaR
approved these changes
Jun 6, 2025
wangxiyuan
force-pushed
the
fix_ascend_config
branch
5 times, most recently
from
830e333 to
8cf5adf
Compare
Signed-off-by: wangxiyuan <wangxiyuan1007@gmail.com>
wangxiyuan
force-pushed
the
fix_ascend_config
branch
from
8cf5adf to
755edcf
Compare
venus-taibai
pushed a commit
to venus-taibai/vllm-ascend
that referenced
this pull request
Jun 18, 2025
…llm-project#1092) Merge branch wengang/cherry-pick-1029-1092 of git@code.alipay.com:Theta/vllm-ascend.git into dev-v0.9.0604 https://code.alipay.com/Theta/vllm-ascend/pull_requests/108 Reviewed-by: 子宏 <tanzhiqiang.tzq@antgroup.com> * [Misc] Refactor additional_config (vllm-project#1029) * [BugFix] Fix ascend config check (vllm-project#1092) * [Misc] Update benchmark scripts
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix the ascend config check logic: