-
-
Couldn't load subscription status.
- Fork 10.9k
[Misc] Remove qlora_adapter_name_or_path #17699
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
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
|
LGTM then |
| load_group.add_argument( | ||
| "--qlora-adapter-name-or-path", | ||
| type=str, | ||
| default=None, | ||
| help="The `--qlora-adapter-name-or-path` has no effect, do not set" | ||
| " it, and it will be removed in v0.10.0.", | ||
| ) |
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.
You could also add deprecated=True here (you'd probably need to merge from main though)
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.
Done
|
|
||
| else: | ||
| model_name_or_path = model_config.model | ||
| model_name_or_path = model_config.model |
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.
Is this variable necessary at all now? We access the other attributes from model_config directly everywhere else in this method.
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.
Done
Signed-off-by: Jee Jee Li <pandaleefree@gmail.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
|
@hmellor It looks like we can't add |
|
Oh, sorry about that, I thought #17426 had merged |
|
It doesn't matter, let's wait for #17426 |
|
Ok it's merged |
|
I'll just force merge this |
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com> Signed-off-by: Mu Huai <tianbowen.tbw@antgroup.com>
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com> Signed-off-by: Yuqi Zhang <yuqizhang@google.com>
qlora_adapter_name_or_pathis a redundant argument and is now deprecated