Skip to content

Conversation

MengqingCao
Copy link
Collaborator

@MengqingCao MengqingCao commented Aug 21, 2025

What this PR does / why we need it?

  1. use action/checkout@v5 instead of v4
  2. remove dbo test case because there is issue with it and will be refactored later
  3. make vllm-ascend compatible with vllm v0.10.1.1 and add CI for it
  4. fix sampler api changes introduced by [Sampler] Support returning final logprobs vllm#22387
  5. fix qwen3 moe config changes intruoduced by [Feature] use --eplb_config to set eplb param vllm#20562
  6. fix kvcache block changes introduced by [Optimization] Make new_block_ids None if empty vllm#23262

Does this PR introduce any user-facing change?

N/A

How was this patch tested?

CI passed with existing test.

Signed-off-by: MengqingCao <cmq0113@163.com>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes a DBO test case to fix the CI pipeline. While this unblocks the CI, I have a high-severity concern about completely deleting the test. Instead, I recommend using @pytest.mark.skip to temporarily disable it. This approach preserves the test code, makes it clear that it's a temporary measure, and helps ensure it won't be forgotten. Please see my detailed comment.

Copy link

👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:‌‌

  • A PR should do only one thing, smaller PRs enable faster reviews.
  • Every PR should include unit tests and end-to-end tests ‌to ensure it works and is not broken by other future PRs.
  • Write the commit message by fulfilling the PR description to help reviewer and future developers understand.

If CI fails, you can run linting and testing checks locally according Contributing and Testing.

vllm_model.generate_greedy(example_prompts, max_tokens)


@patch.dict(os.environ, {"VLLM_ASCEND_ENABLE_DBO": "1"})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skip it instead of removing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there still exsits test_models_distributed_DeepSeekV3_dbo which is skiped below, I think it is duplicated to test on deepseek-v2-lite

 - fix kvcache block changes
 - maintain v0.10.1.1

Signed-off-by: MengqingCao <cmq0113@163.com>
Signed-off-by: MengqingCao <cmq0113@163.com>
Signed-off-by: MengqingCao <cmq0113@163.com>
Signed-off-by: MengqingCao <cmq0113@163.com>
Signed-off-by: MengqingCao <cmq0113@163.com>
Copy link

codecov bot commented Aug 21, 2025

Codecov Report

❌ Patch coverage is 70.11494% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.74%. Comparing base (1de16ea) to head (95b640e).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
tests/ut/core/test_scheduler.py 76.19% 10 Missing ⚠️
vllm_ascend/core/scheduler.py 66.66% 6 Missing ⚠️
vllm_ascend/sample/sampler.py 72.22% 5 Missing ⚠️
vllm_ascend/models/qwen3_moe.py 20.00% 4 Missing ⚠️
tests/ut/kv_connector/utils.py 75.00% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (70.11%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2464      +/-   ##
==========================================
+ Coverage   77.37%   77.74%   +0.37%     
==========================================
  Files         128      132       +4     
  Lines       16455    17519    +1064     
==========================================
+ Hits        12732    13621     +889     
- Misses       3723     3898     +175     
Flag Coverage Δ
unittests 77.74% <70.11%> (+0.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: MengqingCao <cmq0113@163.com>
Signed-off-by: MengqingCao <cmq0113@163.com>
@MengqingCao
Copy link
Collaborator Author

Let's merge this quickly to unblock CI

  • doctest failed due to the wrong version of vllm in the image, will be fixed automatically after this pr merged.
  • ut failed due to an occasional problem, will fix it later in next pr

@wangxiyuan wangxiyuan merged commit b0403f8 into vllm-project:main Aug 21, 2025
30 of 41 checks passed
wangxiaoteng888 pushed a commit to LCAIZJ/vllm-ascend that referenced this pull request Sep 25, 2025
### What this PR does / why we need it?
1. use action/checkout@v5 instead of v4
2. remove dbo test case because there is issue with it and will be
refactored later
3. make vllm-ascend compatible with vllm v0.10.1.1 and add CI for it
4. fix sampler api changes introduced by
vllm-project/vllm#22387
6. fix qwen3 moe config changes intruoduced by
vllm-project/vllm#20562
7. fix kvcache block changes introduced by
vllm-project/vllm#23262

### Does this PR introduce _any_ user-facing change?
N/A

### How was this patch tested?
CI passed with existing test.


- vLLM version: v0.10.0
- vLLM main:
vllm-project/vllm@0c6e40b

---------

Signed-off-by: MengqingCao <cmq0113@163.com>
chopper0126 pushed a commit to chopper0126/vllm-ascend that referenced this pull request Sep 26, 2025
### What this PR does / why we need it?
1. use action/checkout@v5 instead of v4
2. remove dbo test case because there is issue with it and will be
refactored later
3. make vllm-ascend compatible with vllm v0.10.1.1 and add CI for it
4. fix sampler api changes introduced by
vllm-project/vllm#22387
6. fix qwen3 moe config changes intruoduced by
vllm-project/vllm#20562
7. fix kvcache block changes introduced by
vllm-project/vllm#23262

### Does this PR introduce _any_ user-facing change?
N/A

### How was this patch tested?
CI passed with existing test.


- vLLM version: v0.10.0
- vLLM main:
vllm-project/vllm@0c6e40b

---------

Signed-off-by: MengqingCao <cmq0113@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants