[LoRA] minor fix for load_lora_weights() for Flux and a test
#11595
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. |
|
hi, thank you so much for the fast fix. i have verified it resolved the issue. |
| @@ -2545,14 +2545,13 @@ def _maybe_expand_lora_state_dict(cls, transformer, lora_state_dict): | |||
| if unexpected_modules: | |||
| logger.debug(f"Found unexpected modules: {unexpected_modules}. These will be ignored.") | |||
|
|
|||
| is_peft_loaded = getattr(transformer, "peft_config", None) is not None | |||
There was a problem hiding this comment.
Okay to merge. Just a question. Why is this condition causing the issue?
There was a problem hiding this comment.
delete_adapters() doesn't permanently delete the adapter layers. So, as an effect of that even if there's no peft_config in the base model, the base model state dict has base_layer substring present inside of it. Cc: @BenjaminBossan is this expected?
For the case of this PR which fixes #11592, we call load_lora_weights() -> delete_adapters() -> load_lora_weights(), so the said change resolves the problem.
There was a problem hiding this comment.
even if there's no
peft_configin the base model, the base model state dict hasbase_layersubstring present inside of it
Yes, that's expected. PEFT layers wrap the base layer. When deleting adapters, even the last, the PEFT layer does not "unwrap" itself. The only way to achieve that is via peft_model.unload(). Since diffusers does not make use of PeftModel, if this is desired, it would need to be re-implemented.
What does this PR do?
Fixes #11592
Additionally, adds a test to check the following flow of operations work as expected:
load_lora_weights()->delete_adapters()->load_lora_weights().