Conversation
This reverts commit f10acdd.
|
👋 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. |
github-actions
bot
added
documentation
![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 reverts a previous change, reintroducing the ascend_scheduler. The changes primarily involve updating documentation, examples, and tests to reflect this. I've identified a few issues in the configurations within the tests and documentation that need attention. These include a typo in a configuration key and the use of incorrect or redundant keys, which could lead to confusion and incorrect behavior.
| 'enable_chunked_prefill' : False, | ||
| 'chunked_prefill_enabled': False |
There was a problem hiding this comment.
The configuration key chunked_prefill_enabled is incorrect and has no effect. The correct key is enable_chunked_prefill. Please remove the incorrect key and the trailing comma from the preceding line.
| 'enable_chunked_prefill' : False, | |
| 'chunked_prefill_enabled': False | |
| 'enable_chunked_prefill' : False |
| additional_config={ | ||
| "ascend_scheduler_config": { | ||
| "enabled": True, | ||
| "chunked_prefill_enabled": False, |
There was a problem hiding this comment.
The configuration key chunked_prefill_enabled is incorrect. It should be enable_chunked_prefill. Due to this, the test is not running with chunked prefill disabled as intended.
| "chunked_prefill_enabled": False, | |
| "enable_chunked_prefill": False, |
| additional_config={ | ||
| "ascend_scheduler_config": { | ||
| "enabled": True, | ||
| "chunked_prefill_enabled": False, |
There was a problem hiding this comment.
The configuration key chunked_prefill_enabled is incorrect. It should be enable_chunked_prefill. Due to this, the test is not running with chunked prefill disabled as intended.
| "chunked_prefill_enabled": False, | |
| "enable_chunked_prefill": False, |
| if not use_v1_schduler: | ||
| kwargs = { | ||
| "ascend_scheduler_config": { | ||
| "enable": True, |
There was a problem hiding this comment.
There is a typo in the configuration key. "enable" should be "enabled". This causes the ascend scheduler to be disabled by default, and the test does not run with the intended configuration.
| "enable": True, | |
| "enabled": True, |
wangxiyuan
left a comment
wangxiyuan
left a comment
There was a problem hiding this comment.
This change lead CI test oom. Let's revert it first.
Reverts vllm-project#4498 - vLLM version: v0.11.2 - vLLM main: https://github.com/vllm-project/vllm/commit/v0.11.2
Reverts vllm-project#4498 - vLLM version: v0.11.2 - vLLM main: https://github.com/vllm-project/vllm/commit/v0.11.2 Signed-off-by: Che Ruan <cr623@ic.ac.uk>
Reverts vllm-project#4498 - vLLM version: v0.11.2 - vLLM main: https://github.com/vllm-project/vllm/commit/v0.11.2 Signed-off-by: Che Ruan <cr623@ic.ac.uk>
Reverts vllm-project#4498 - vLLM version: v0.11.2 - vLLM main: https://github.com/vllm-project/vllm/commit/v0.11.2
Reverts vllm-project#4498 - vLLM version: v0.11.2 - vLLM main: https://github.com/vllm-project/vllm/commit/v0.11.2 Signed-off-by: tanqingshan (A) <50050625@china.huawei.com>
Reverts vllm-project#4498 - vLLM version: v0.11.2 - vLLM main: https://github.com/vllm-project/vllm/commit/v0.11.2
Reverts vllm-project#4498 - vLLM version: v0.11.2 - vLLM main: https://github.com/vllm-project/vllm/commit/v0.11.2
2 participants
Reverts #4498