[Misc] Fix enable_sequence_parallelism in PassConfig#4767
[Misc] Fix enable_sequence_parallelism in PassConfig#4767MengqingCao wants to merge 1 commit intovllm-project:mainfrom
Conversation
Signed-off-by: MengqingCao <cmq0113@163.com>
There was a problem hiding this comment.
Code Review
This pull request aims to add compatibility for the enable_sequence_parallelism configuration option across different vLLM versions. The changes in vllm_ascend/utils.py and CustomQwen3MoeDecoderLayer correctly implement version checking. However, a similar compatibility logic is missing in CustomQwen3MoeForCausalLM, which could lead to issues with newer vLLM versions. I have provided a suggestion to address this critical issue.
| self.model.make_empty_intermediate_tensors) | ||
|
|
||
| self.enable_sequence_parallelism = vllm_config.compilation_config.pass_config.enable_sp | ||
| self.enable_sequence_parallelism = vllm_config.compilation_config.pass_config.enable_sequence_parallelism |
There was a problem hiding this comment.
This change appears to be incorrect. It replaces enable_sp with enable_sequence_parallelism without the necessary compatibility logic for different vLLM versions. This will likely cause a failure on vLLM versions newer than 0.12.0. To ensure compatibility, you should apply the same version-checking logic that is used in CustomQwen3MoeDecoderLayer.__init__ and vllm_ascend/utils.py.
| self.enable_sequence_parallelism = vllm_config.compilation_config.pass_config.enable_sequence_parallelism | |
| self.enable_sequence_parallelism = ( | |
| vllm_config.compilation_config.pass_config.enable_sequence_parallelism | |
| if vllm_version_is("0.12.0") else vllm_config.compilation_config.pass_config.enable_sp | |
| ) |
|
it's included in v0.12.0. https://github.com/vllm-project/vllm/tree/v0.12.0 I think you should update your local code? |
|
git tag --delete v0.12.0 |
Yes, you're right, thx! |
|
👋 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?
Fix enable_sequence_parallelism in PassConfig
after vllm-project/vllm#29646,
PassConfig.enable_sequence_parallelismis renamed toPassConfig.enable_sp, but it dosen't be included in vLLM v0.12.0, we need to make compatibility for itHow was this patch tested?
CI passed with existing test.