-
Notifications
You must be signed in to change notification settings - Fork 743
[Main2Main] Upgrade vllm commit to 0112 #5691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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 updates the vLLM commit dependency and adapts the codebase for the removal of init_cached_hf_modules in newer vLLM versions. The changes introduce a version check to maintain backward compatibility.
My review focuses on making this compatibility check more robust. I've suggested replacing the version-specific check with a feature detection approach (hasattr), which is a more resilient and maintainable pattern for handling optional functionalities across different library versions. I've also pointed out that the corresponding unit test should be made hermetic by mocking the version check, to ensure it's independent of the installed vLLM version.
|
👋 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. |
40ed13a to
c861250
Compare
5224123 to
c046d9e
Compare
|
Another 4-cards break is due to: vllm-project/vllm#31773 |
c046d9e to
ef8e3f1
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
ef8e3f1 to
62bc1f3
Compare
754b76d to
5f05e08
Compare
.github/workflows/_e2e_test.yaml
Outdated
| # TODO: add ignore after the issue is fixed | ||
| pytest -sv --durations=0 tests/e2e/singlecard/spec_decode/test_mtp_eagle_correctness.py | ||
| pytest -sv --durations=0 tests/e2e/singlecard/spec_decode/test_v1_spec_decode.py | ||
| pytest -sv --durations=0 tests/e2e/singlecard/spec_decode/test_v1_spec_decode.py \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please delete all skip cases, the break points are gone now
| from vllm.attention.backends.abstract import \ | ||
| AttentionMetadata # type: ignore | ||
| else: | ||
| from vllm.v1.attention.backend import AttentionMetadata # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please recheck all changes about this
caf4690 to
f2f9110
Compare
Signed-off-by: wjunLu <[email protected]> Signed-off-by: hfadzxy <[email protected]>
f2f9110 to
125d51b
Compare
|
main2main upgrade to 0113, The pr not need |
What this PR does / why we need it?
Upgrade vllm commit to 0112 (d7b2e57097dae8a620c28eddf663adad2a8329c5)
init_cached_hf_modulesdue to [Chore] Try removeinit_cached_hf_modulesvllm#31786Does this PR introduce any user-facing change?
How was this patch tested?