Skip to content
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

Add LoRA+ to optimizer params #170

Closed
wants to merge 6 commits into from

Conversation

rockerBOO
Copy link
Contributor

Supports LoRA+ in Kohya.

Willing to update the code, values, naming, flow and so on if required.

Screenshot 2024-03-25 at 13-55-27 2402 12354 pdf
Screenshot 2024-03-25 at 16-20-08 2402 12354 pdf

I was guessing at the values for LoHa and LoKr which I think is accurate and works according to my testing. Haven't tried like GLoRA yet.

Related PR for Kohya-ss: kohya-ss/sd-scripts#1233

https://arxiv.org/abs/2402.12354
https://github.com/nikhil-ghosh-berkeley/loraplus

@KohakuBlueleaf
Copy link
Owner

  1. Please use old method/param groups when lora_plus_ratio is None
  2. Please remove commented code

@rockerBOO
Copy link
Contributor Author

  1. Please use old method/param groups when lora_plus_ratio is None

What do you mean by use the old method/param groups? You want 2 methods for prepare_optimizer_params where it is the current and lora_plus_ratio version separated?

Or you mean the lora/plus "naming"? Those are not used past the original separation. Could be combined instead of 2 steps.

Will remove the commented out code, and have some bug fixes coming as well. Thank you.

@rockerBOO rockerBOO marked this pull request as draft April 4, 2024 21:06
@KohakuBlueleaf
Copy link
Owner

  1. Please use old method/param groups when lora_plus_ratio is None

What do you mean by use the old method/param groups? You want 2 methods for prepare_optimizer_params where it is the current and lora_plus_ratio version separated?

Or you mean the lora/plus "naming"? Those are not used past the original separation. Could be combined instead of 2 steps.

Will remove the commented out code, and have some bug fixes coming as well. Thank you.

Basically
plz ensure your code can work with "None" as input of lora_plus params.
For now it will not work.

And what I want is basically: send single param group (or 2~3 for UNet/TE1/TE2) when lora_plus params are None

@rockerBOO
Copy link
Contributor Author

Made some fixes and added GLoRA. Should now properly support single param group, only TE and only UNet and all LoRA+ and LR permutations.

The following test script I made to make sure it is working as expected. Willing to fix anything you find incorrect.

https://gist.github.com/rockerBOO/7e58b268d266c6f55a7f90085a1d1dfe

@rockerBOO rockerBOO marked this pull request as ready for review April 11, 2024 21:33
@rockerBOO rockerBOO marked this pull request as draft April 30, 2024 20:13
@rockerBOO
Copy link
Contributor Author

Being reworked on Kohya's side a little bit. Will rehash this once that is complete.

@rockerBOO rockerBOO closed this May 21, 2024
@rockerBOO rockerBOO deleted the lora_plus branch May 21, 2024 19:22
@rockerBOO rockerBOO restored the lora_plus branch May 21, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants