-
-
Notifications
You must be signed in to change notification settings - Fork 11k
[V0 deprecation] Remove _VLLM_V1 suffixes from attention backend names #25489
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
[V0 deprecation] Remove _VLLM_V1 suffixes from attention backend names #25489
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 effectively removes the _VLLM_V1 suffixes from attention backend names, which is a great cleanup now that V0 backends are deprecated. The changes are consistent across the codebase, covering test files, configuration, and backend implementations. I appreciate the addition of backward compatibility in vllm/attention/selector.py to handle the old suffixes from environment variables with a warning. I've found one minor area for improvement to simplify a redundant conditional check.
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.
This is a breaking change right? We should put in a deprecation notice + fallback for at least a release
| if backend_by_env_var.endswith("_VLLM_V1"): | ||
| logger.warning( | ||
| "The suffix '_VLLM_V1' in the environment variable " | ||
| "%s is no longer necessary as V0 backends have been " | ||
| "deprecated. Please remove this suffix from your " | ||
| "environment variable setting.", STR_BACKEND_ENV_VAR) | ||
| backend_by_env_var = backend_by_env_var.removesuffix( | ||
| "_VLLM_V1") |
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.
@tlrmchlsmth It shouldn't be breaking; these lines should prevent issues
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.
perfect
|
Failures are related, PTAL |
…ction logic) Signed-off-by: Matthew Bonanni <mbonanni001@gmail.com>
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
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.
LGTM with the test changes, thanks a ton!
|
The v1 tests look related too |
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Head branch was pushed to by a user without write access
|
@mgoin Thanks - I removed that test case too (there are no longer any V0 backends to fall back to). Eventually we'll have to remove |
|
Created #25673 to remove the oracle |
|
@mgoin V1 Test e2e + engine is failing for backend |
|
@MatthewBonanni is it possible to fix the test? Otherwise you can make an issue and disable it |
|
@ProExpertProg It looks to me like a correctness issue with the FI backend outside the scope of this PR, so I'll disable it and make an issue. Disabling it shouldn't do too much harm because it was effectively disabled before. EDIT: The issue is #25679 |
vllm-project/vllm#25489 Signed-off-by: Chendi Xue <Chendi.Xue@intel.com>
vllm-project/vllm#25489 Signed-off-by: Chendi Xue <Chendi.Xue@intel.com> Signed-off-by: Iryna Boiko <iboiko@habana.ai>
#25489) Signed-off-by: Matthew Bonanni <mbonanni@redhat.com> Signed-off-by: Matthew Bonanni <mbonanni001@gmail.com> Signed-off-by: yewentao256 <zhyanwentao@126.com>
vllm-project#25489) Signed-off-by: Matthew Bonanni <mbonanni@redhat.com> Signed-off-by: Matthew Bonanni <mbonanni001@gmail.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
vllm-project#25489) Signed-off-by: Matthew Bonanni <mbonanni@redhat.com> Signed-off-by: Matthew Bonanni <mbonanni001@gmail.com>
vllm-project#25489) Signed-off-by: Matthew Bonanni <mbonanni@redhat.com> Signed-off-by: Matthew Bonanni <mbonanni001@gmail.com>
vllm-project#25489) Signed-off-by: Matthew Bonanni <mbonanni@redhat.com> Signed-off-by: Matthew Bonanni <mbonanni001@gmail.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
Purpose
Remove
_VLLM_V1suffixes as these are no longer needed. If a user includes that suffix in theirVLLM_ATTENTION_BACKENDenvironment variable, it is stripped and a warning is printed.Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.