-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
[LoRA] fix adapter movement when using DoRA. #9411
Conversation
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
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.
Ah yes, nice catch, LGTM.
is this test supposed to fail at main right now? (after replace the checkpoint)? |
Yes, it'd fail with the replacement. However, previously, it wasn't the case. @BenjaminBossan I wonder if something changed on the |
Sorry, I'm missing some context. Under what circumstances is the test failing? Are there logs? |
Sorry, what I meant is that the test The failure: # We will offload the first adapter in CPU and check if the offloading
# has been performed correctly
> pipe.set_lora_device(["adapter-1"], "cpu")
tests/lora/test_lora_layers_sd.py:126:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
src/diffusers/loaders/lora_base.py:702: in set_lora_device
module.lora_magnitude_vector[adapter_name] = module.lora_magnitude_vector[
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = ModuleDict(), key = 'adapter-1'
@_copy_to_script_wrapper
def __getitem__(self, key: str) -> Module:
> return self._modules[key]
E KeyError: 'adapter-1' But with |
I see, thanks for the extra info. Indeed, this PR broke the test: huggingface/peft#1806. It's been merged for quite some time, not sure why we didn't notice earlier, is it because the SD model was removed? Anyway, the fix in this PR is correct given the change in PEFT. |
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.
thanks!
fix adapter movement when using DoRA.
What does this PR do?
To confirm, run:
pytest tests/lora/test_lora_layers_sd.py::StableDiffusionLoRATests::test_integration_move_lora_cpu
after replacing "runwayml/stable-diffusion-v1-5" with "Jiali/stable-diffusion-1.5" in thetests/lora/test_lora_layers_sd.py
file.