Skip to content

[Misc] Reduce LoRA-related static variable #13166

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
Feb 22, 2025
Merged

[Misc] Reduce LoRA-related static variable #13166

merged 15 commits into from
Feb 22, 2025

Conversation

@jeejeelee
Copy link
Collaborator

@jeejeelee jeejeelee commented Feb 12, 2025 •
edited by github-actions bot
Loading

Motivation

Remove LoRA-related static variable supported_lora_modules, which not only makes our model implementation cleaner but also enables smoother LoRA support

Work

  • Delete all models supported_lora_modules
  • Add unit test

@jeejeelee
Init
75619ed 
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
@jeejeelee jeejeelee marked this pull request as draft February 12, 2025 16:34
@github-actions
Copy link

github-actions bot commented Feb 12, 2025

👋 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 fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

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 ready label to the PR or enable auto-merge.

🚀

@jeejeelee jeejeelee mentioned this pull request Feb 13, 2025
Closed
@jeejeelee
Backup
a5fefce 
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
@jeejeelee
Merge branch 'vllm-project:main' into delete-supported-lora-modules
bfeaf67
@jeejeelee
Delete models var
33e06c5 
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
@jeejeelee
Merge branch 'vllm-project:main' into delete-supported-lora-modules
eb2bab4
@jeejeelee
Modify unit test
e12c3db 
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
@jeejeelee jeejeelee force-pushed the delete-supported-lora-modules branch from 7e5ad01 to e12c3db Compare February 17, 2025 16:39
@hmellor hmellor mentioned this pull request Feb 17, 2025
Closed
@jeejeelee
Delete remainning lora var
25584d3 
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
@jeejeelee
Delete unused code
747fc07 
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
@jeejeelee
Move forward
b605d10 
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
@jeejeelee
Add LoRA for transformers model
fbb7558 
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
@jeejeelee jeejeelee force-pushed the delete-supported-lora-modules branch from 44665c6 to fbb7558 Compare February 20, 2025 16:34
@mergify
Copy link

mergify bot commented Feb 20, 2025

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @jeejeelee.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Feb 20, 2025
@mergify mergify bot removed the needs-rebase label Feb 20, 2025
@jeejeelee
Done
3ce2624 
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
@jeejeelee jeejeelee force-pushed the delete-supported-lora-modules branch from 5190aee to 3ce2624 Compare February 21, 2025 02:43
@jeejeelee jeejeelee marked this pull request as ready for review February 21, 2025 02:44
jeejeelee
jeejeelee commented Feb 21, 2025
View reviewed changes
vllm/model_executor/models/transformers.py
"""
return lora_linear_cls[vllm_linear_cls][fully_sharded]

return HFCompatibleLinear(
Copy link
Collaborator Author

@jeejeelee jeejeelee Feb 21, 2025

Choose a reason for hiding this comment

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

After completing this PR, transformers-model will directly support LoRA, but there are some issues with inference that are expected to be resolved in subsequent PRs.

@jeejeelee jeejeelee requested review from DarkLight1337 and Isotr0py and removed request for Isotr0py February 21, 2025 02:49
Isotr0py
Isotr0py approved these changes Feb 21, 2025
View reviewed changes
Copy link
Member

@Isotr0py Isotr0py left a comment

Choose a reason for hiding this comment

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

Tests passed on my side locally, LGTM!

@jeejeelee jeejeelee added the ready ONLY add when PR is ready to merge/full CI is needed label Feb 21, 2025
@jeejeelee
Copy link
Collaborator Author

jeejeelee commented Feb 22, 2025

@DarkLight1337 Do you know what's causing the current CI failures?

@Isotr0py
Copy link
Member

Isotr0py commented Feb 22, 2025

There were some issues in HF's opt repo yesterday, which should have been fixed. I think re-run these CIs should be just fine?

@DarkLight1337 DarkLight1337 enabled auto-merge (squash) February 22, 2025 08:21
@simon-mo simon-mo merged commit 105b8ce into vllm-project:main Feb 22, 2025
54 of 66 checks passed
@jeejeelee jeejeelee deleted the delete-supported-lora-modules branch February 23, 2025 03:44
@hmellor hmellor mentioned this pull request Feb 24, 2025
Closed
This was referenced Feb 25, 2025
Merged
Merged
@hmellor hmellor mentioned this pull request Mar 3, 2025
Merged
Akshat-Tripathi pushed a commit to krai/vllm that referenced this pull request Mar 3, 2025
@jeejeelee @Akshat-Tripathi
[Misc] Reduce LoRA-related static variable (vllm-project#13166)
e4f5b9c
lulmer pushed a commit to lulmer/vllm that referenced this pull request Apr 7, 2025
@jeejeelee @lulmer
[Misc] Reduce LoRA-related static variable (vllm-project#13166)
ce6b7fe 
Signed-off-by: Louis Ulmer <ulmerlouis@gmail.com>
@ckhordiasma ckhordiasma mentioned this pull request Apr 17, 2025
Closed
shreyankg pushed a commit to shreyankg/vllm that referenced this pull request May 3, 2025
@jeejeelee @shreyankg
[Misc] Reduce LoRA-related static variable (vllm-project#13166)
dc83e37
@paulyu12 paulyu12 mentioned this pull request May 30, 2025
Merged
wangxiyuan pushed a commit to vllm-project/vllm-ascend that referenced this pull request Jun 3, 2025
@paulyu12
[V0.7.3][LoRA][Qwen3] Make v0.7.3 support Qwen3+LoRA (#1037)
b69d41d 
### What this PR does / why we need it?
Because the EOL vLLM-v0.7.3 lacks this
PR(vllm-project/vllm#13166), while launching
Qwen3+LoRA on vllm-ascend0.7.3, the error **"Qwen3ForCausalLM" object
has no attribute "embedding modules**" will be raised.

We modify qwen3.py to support Qwen3+LoRA on vllm-ascend v0.7.3 instead.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?

---------

Signed-off-by: paulyu <paulyu0307@gmail.com>
Co-authored-by: paulyu <paulyu0307@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Isotr0py Isotr0py Isotr0py approved these changes

@DarkLight1337 DarkLight1337 Awaiting requested review from DarkLight1337

Assignees

No one assigned

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jeejeelee @Isotr0py @simon-mo