[vllm] dynamically switch lora request#3933
[vllm] dynamically switch lora request#3933Datta0 wants to merge 3 commits intounslothai:nightlyfrom
Conversation
for more information, see https://pre-commit.ci
Summary of ChangesHello @Datta0, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the vLLM integration by introducing dynamic LoRA (Low-Rank Adaptation) request handling. It modifies the model's generation functions to automatically detect updates to LoRA parameters and reload the corresponding LoRA weights on the fly. This allows for seamless switching between different fine-tuned LoRA adapters during inference without requiring explicit manual configuration, improving flexibility and ease of use for models utilizing LoRA with vLLM. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces dynamic LoRA request switching for vLLM-based fast inference. The implementation correctly detects changes in LoRA weights and reloads them by patching the fast_generate and fast_generate_batches methods. My review includes suggestions to improve robustness by adding logging to exception handling, and to improve code style and performance by moving inline imports and removing unnecessary pass statements.
|
|
||
| from unsloth_zoo.vllm_utils import load_lora | ||
|
|
||
| self._unsloth_cached_lora_request = load_lora( |
There was a problem hiding this comment.
The try...except Exception as e: block is too broad and silently fails by returning None. This can hide important errors during LoRA loading, making it difficult to debug why adapters might not be applying correctly. It's recommended to at least log the exception to provide visibility into potential issues.
| self._unsloth_cached_lora_request = load_lora( | |
| print(f"Unsloth: Failed to get LoRA request: {e}") | |
| return None |
| ) | ||
|
|
||
| if not hasattr(self, "_unsloth_lora_id"): | ||
| self._unsloth_lora_id = 0 |
| self._unsloth_lora_temp_dir = tempfile.TemporaryDirectory() | ||
|
|
||
| # Update if changed | ||
| if ( |
There was a problem hiding this comment.
This inline import of load_lora from unsloth_zoo.vllm_utils is inside a conditional block that is executed frequently if LoRA weights change. To avoid the overhead of re-importing the module repeatedly, it's better to move this import statement to a higher scope, for example, at the beginning of the _get_lora_request function, before the conditional check.
| from unsloth_zoo.vllm_utils import load_lora | ||
|
|
||
| self._unsloth_cached_lora_request = load_lora( | ||
| self, |
| ) | ||
| return self._unsloth_cached_lora_request | ||
| except Exception as e: | ||
| return None |
| def _fast_generate_patched(self, *args, **kwargs): | ||
| if "lora_request" not in kwargs: | ||
| req = _get_lora_request(self) | ||
| if req is not None: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a346034817
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # Update if changed | ||
| if ( | ||
| current_version != self._unsloth_lora_version | ||
| or self._unsloth_cached_lora_request is None | ||
| ): |
There was a problem hiding this comment.
Invalidate LoRA cache when active adapter changes
The cached lora_request is only refreshed when the sum of trainable parameter versions changes. Switching adapters via PEFT (e.g., model.set_adapter(...) / active_adapters updates) typically does not mutate parameter tensors, so current_version stays the same and the cached request is reused. In that scenario, fast_generate keeps sending the old adapter’s LoRA to vLLM, producing outputs for the wrong adapter. Consider including the active adapter name(s) (or a hash of relevant PEFT config) in the cache key so adapter switches invalidate the request.
Useful? React with 👍 / 👎.
Requires: unslothai/unsloth-zoo#445