Add lora+ implentation #1509

moghadas76 · 2024-02-26T15:55:40Z

Implementing LoRA+ https://arxiv.org/abs/2402.12354

BenjaminBossan · 2024-02-26T16:39:48Z

Duplicate of #1504 :)

Sorry about closing (wrong button).

HuggingFaceDocBuilderDev · 2024-02-26T16:44:21Z

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.

moghadas76 · 2024-02-26T16:45:25Z

What was the conclusion in that issue?

BenjaminBossan · 2024-02-26T17:10:22Z

No conclusion yet, we want to wait and see if the performance gains are indeed robust. Regarding your code, it's basically just a giant string with the code, right? Was that the intent?

moghadas76 · 2024-02-26T17:12:58Z

waiting for you to ask implement new Trainer object or not?

BenjaminBossan · 2024-03-06T10:58:16Z

Hey, after some discussion, I think we can proceed with this project. Let's add the create_loraplus_optimizer function but not the custom trainer class. We can put the function inside of peft/helpers.py.

Some considerations:

Add a reference to the original repo
Update the docs
Remove the logger code
If you feel up for the task, let's add some unit tests.

BenjaminBossan · 2024-03-12T12:41:36Z

@moghadas76 do you still plan on working on this?

moghadas76 · 2024-03-12T12:42:53Z

Yes, This weekend I'll fix the points

…

On Tue, Mar 12, 2024, 1:41 PM Benjamin Bossan ***@***.***> wrote: @moghadas76 <https://github.com/moghadas76> do you still plan on working on this? — Reply to this email directly, view it on GitHub <#1509 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFRH3KKIYBNPIB4W5RLD5PTYX3ZZPAVCNFSM6AAAAABD2OVEOGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJRGU3DGMZQHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

BenjaminBossan · 2024-03-12T12:54:51Z

Great, thanks. On top of what I mentioned, let's also move this to a new file. I'm thinking src/peft/optimizers/loraplus.py. The idea here is that we want to add more optimizer-related methods in the future, so it makes sense to choose a proper file structure right away.

moghadas76 · 2024-03-17T16:22:03Z

Please review my code

BenjaminBossan

Thanks for working on this. It is a good start but there are a few issues, please check my comments. On top of that, could you please move the function out of helpers.py into a separate module, as I mentioned above?

Great, thanks. On top of what I mentioned, let's also move this to a new file. I'm thinking src/peft/optimizers/loraplus.py. The idea here is that we want to add more optimizer-related methods in the future, so it makes sense to choose a proper file structure right away.

Moreover, it would be great to document this function in our PEFT docs, but it would be fine to do that in a follow-up PR.

Finally, please run make style on your changes.

src/peft/tuners/lora/config.py

src/peft/helpers.py

tests/test_loraplus_helper.py

src/peft/helpers.py

BenjaminBossan · 2024-03-25T11:48:02Z

@moghadas76 Do you still plan on working on this?

moghadas76 · 2024-03-25T11:51:30Z

Yes, I'll fix the comments tonight

…

On Mon, Mar 25, 2024, 12:48 PM Benjamin Bossan ***@***.***> wrote: @moghadas76 <https://github.com/moghadas76> Do you still plan on working on this? — Reply to this email directly, view it on GitHub <#1509 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFRH3KIOTROBBMBYUMYCHE3Y2AFIPAVCNFSM6AAAAABD2OVEOGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJXHAZDIOBVGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

BenjaminBossan · 2024-03-25T11:56:40Z

Yes, I'll fix the comments tonight

Thanks. No need to rush, I just wanted to inquire if you're still on it :)

BenjaminBossan · 2024-04-02T12:44:51Z

@moghadas76 LMK once you're finished with your changes and want me to do another review.

BenjaminBossan · 2024-04-19T09:53:12Z

Gentle ping @moghadas76

moghadas76 · 2024-04-19T20:01:41Z

Hi
I fixed the comments
Please review again

moghadas76 · 2024-04-19T20:02:00Z

@BenjaminBossan

BenjaminBossan · 2024-04-25T10:07:50Z

Sorry for the delay, I was at a conference, will review soon.

BenjaminBossan

Thanks for making the adjustments, this already looks quite good. I still found a few minor areas for improvements, which I commented. Also, as mentioned in my earlier comment, could you please move the code to a different file?

src/peft/utils/peft_types.py

src/peft/helpers.py

tests/test_loraplus_helper.py

BenjaminBossan · 2024-05-21T13:40:14Z

Hmm, code quality checks are still failing with:

tests/test_loraplus_helper.py:1:1: I001 [*] Import block is un-sorted or un-formatted

Is it possible that your local ruff version differs? CI uses v0.2.2.

moghadas76 · 2024-05-21T20:44:55Z

You were right. My ruff version was old.

BenjaminBossan

Thanks for the updates. Our code style check still fails though, not sure what the reason is if you use the same ruff version. Here is the diff that I get when running ruff locally on your branch:

modified   src/peft/optimizers/__init__.py
@@ -17,4 +17,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from .loraplus import create_loraplus_optimizer
\ No newline at end of file
+from .loraplus import create_loraplus_optimizer
modified   src/peft/optimizers/loraplus.py
@@ -8,20 +8,24 @@ from transformers.trainer_pt_utils import get_parameter_names
 from ..peft_model import PeftModel
 
 
-def create_loraplus_optimizer(model: PeftModel, optimizer_cls: type[Optimizer], optimizer_kwargs: dict, loraplus_lr_embedding: float=1e-6) -> Optimizer:
+def create_loraplus_optimizer(
+    model: PeftModel, optimizer_cls: type[Optimizer], optimizer_kwargs: dict, loraplus_lr_embedding: float = 1e-6
+) -> Optimizer:
     """
-    Creates a LoraPlus optimizer.
-    Implementing LoRA+ https://arxiv.org/abs/2402.12354
-    Reference: https://github.com/nikhil-ghosh-berkeley/loraplus/
+    Creates a LoraPlus optimizer. Implementing LoRA+ https://arxiv.org/abs/2402.12354 Reference:
+    https://github.com/nikhil-ghosh-berkeley/loraplus/
 
     Args:
         model (`torch.nn.Module`): The model to be optimized.
         optimizer_cls (`torch.optim.Optimizer`): The optimizer class to be used.
         optimizer_kwargs (`dict`): Additional keyword arguments to be passed to the optimizer.
-            - **loraplus_lr_ratio** (`float`): The ratio of the learning rate to be used for the embedding layer. Defaults to loraplus_lr_ratio
-            - loraplus_lr_embedding (`float`): The learning rate to be used for the embedding layer. Defaults to loraplus_lr_embedding
+            - **loraplus_lr_ratio** (`float`): The ratio of the learning rate to be used for the embedding layer.
+              Defaults to loraplus_lr_ratio
+            - loraplus_lr_embedding (`float`): The learning rate to be used for the embedding layer. Defaults to
+              loraplus_lr_embedding
     """
     from ..tuners.lora.layer import Embedding
+
     loraplus_lr_ratio = optimizer_kwargs.pop("loraplus_lr_ratio")
 
     decay_parameters = get_parameter_names(model, ALL_LAYERNORM_LAYERS)
@@ -81,6 +85,7 @@ def create_loraplus_optimizer(model: PeftModel, optimizer_cls: type[Optimizer],
     optimizer = optimizer_cls(optimizer_grouped_parameters, **optimizer_kwargs)
     if optimizer_cls.__name__ == "Adam8bit":
         import bitsandbytes
+
         manager = bitsandbytes.optim.GlobalOptimManager.get_instance()
         for module in model.modules():
             if isinstance(module, nn.Embedding):
modified   tests/test_loraplus_helper.py
@@ -25,32 +25,37 @@ def test_lora_plus_helper_sucess():
     model = SimpleNet()
     optimizer_cls = bnb.optim.Adam8bit
     optim_config = {
-        'lr': 5e-5,
-        'eps': 1e-6,
-        'betas': (0.9, 0.999),
-        'weight_decay': 0.0,
+        "lr": 5e-5,
+        "eps": 1e-6,
+        "betas": (0.9, 0.999),
+        "weight_decay": 0.0,
         "loraplus_lr_ratio": 0.2,
     }
-    optim = create_loraplus_optimizer(model=model, optimizer_cls=optimizer_cls, optimizer_kwargs=optim_config, loraplus_lr_embedding=1e-6)
+    optim = create_loraplus_optimizer(
+        model=model, optimizer_cls=optimizer_cls, optimizer_kwargs=optim_config, loraplus_lr_embedding=1e-6
+    )
     assert optim is not None
     assert len(optim.param_groups) == 4
 
+
 def test_lora_plus_optimizer_sucess():
     optimizer_cls = bnb.optim.Adam8bit
     optim_config = {
-        'lr': 5e-5,
-        'eps': 1e-6,
-        'betas': (0.9, 0.999),
-        'weight_decay': 0.0,
+        "lr": 5e-5,
+        "eps": 1e-6,
+        "betas": (0.9, 0.999),
+        "weight_decay": 0.0,
         "loraplus_lr_ratio": 0.2,
     }
     model: SimpleNet = SimpleNet().cuda()
-    optim = create_loraplus_optimizer(model=model, optimizer_cls=optimizer_cls, optimizer_kwargs=optim_config, loraplus_lr_embedding=1e-6)
+    optim = create_loraplus_optimizer(
+        model=model, optimizer_cls=optimizer_cls, optimizer_kwargs=optim_config, loraplus_lr_embedding=1e-6
+    )
     loss = torch.nn.CrossEntropyLoss()
     bnb.optim.GlobalOptimManager.get_instance().register_parameters(model.parameters())
     x = torch.randint(100, (2, 4, 10)).cuda()
     output = model(x).permute(0, 3, 1, 2)
-    label = torch.randint(16, (2,4,10,)).cuda()
+    label = torch.randint(16, (2, 4, 10)).cuda()
     loss_value = loss(output, label)
     loss_value.backward()
     optim.step()

tests/test_loraplus_helper.py

moghadas76 · 2024-06-09T16:16:25Z

Could you determine what is the problem of
doc-builder style src/peft tests docs/source --max_len 119 --check_only

Traceback (most recent call last):
File "/home/moghadas/miniconda3/bin/doc-builder", line 8, in
sys.exit(main())
^^^^^^
File "/home/moghadas/miniconda3/lib/python3.11/site-packages/doc_builder/commands/doc_builder_cli.py", line 47, in main
args.func(args)
File "/home/moghadas/miniconda3/lib/python3.11/site-packages/doc_builder/commands/style.py", line 28, in style_command
raise ValueError(f"{len(changed)} files should be restyled!")
ValueError: 2 files should be restyled!

BenjaminBossan · 2024-06-10T09:55:48Z

Could you determine what is the problem of
doc-builder style src/peft tests docs/source --max_len 119 --check_only

Where did you see that? The code quality check only spits out this:

tests/test_loraplus_helper.py:52:56: W291 Trailing whitespace

Also, these lines could probably be reduced to a single line:

https://github.com/huggingface/peft/pull/1509/files#diff-4d3762210943c647e8ba391b5261730402551c7fa8f3903686fdabde648cea89R71-R78

I guess what happened here is that your editor added those line breaks because it is configured with a lower line limit than what we have in PEFT.

shubhamjain0594 · 2024-06-23T10:46:42Z

@moghadas76 @BenjaminBossan I can take this up and make necessary changes if you are short on time. We can aim to get this PR merged this week, let me know if its okay.

BenjaminBossan · 2024-06-24T10:46:28Z

I can take this up and make necessary changes if you are short on time. We can aim to get this PR merged this week, let me know if its okay.

That's fine from my point of view. A separate PR with credits given would also work for me.

For my understanding: Who is "we" in this case, are you collaborating with moghadas76?

shubhamjain0594 · 2024-06-24T11:25:24Z

That's fine from my point of view. A separate PR with credits given would also work for me.

For my understanding: Who is "we" in this case, are you collaborating with moghadas76?

I am doing it by myself. By "we" I just meant you and me, and @moghadas76 if they are available.

Also can you advice on how to provide the credit?

BenjaminBossan · 2024-06-24T12:37:34Z

I see. Sure, please go ahead. As you probably can't push on top of this PR, feel free to create a new one. If we don't hear back from moghadas76 by the time the new PR is ready to be merged, we can add them as a co-author.

stillmatic · 2024-07-01T13:58:59Z

I am happy to clean this up too.

IMO the API is not the most clear as currently presented. IMO the embedding LR and the ratio should be either both optimizer_kwargs or both named args. It makes more sense to me that the optimizer kwargs are purely passed to the optimizer rather than to the LR adjustments.

Finally, should the 8 bit -> 32 bit upcast be applied to all the 8 bit optimizers?

    eight_bit_names = ["Adam8bit", "AdamW8bit", "PagedAdam8bit", "PagedAdamW8bit"]
    if optimizer_cls.__name__ in eight_bit_names:

BenjaminBossan · 2024-07-01T15:00:07Z

@stillmatic Thanks, that would also be fine, just pinging @shubhamjain0594 to ensure that there won't be any duplicate work.

moghadas76 · 2024-07-01T16:38:26Z

This is very disrespectful. He stole this branch.

shubhamjain0594 · 2024-07-01T20:52:38Z

This is very disrespectful. He stole this branch.

@BenjaminBossan if @stillmatic has time then sure please go for it. I was going to raise a PR today, but was mainly looking to fix some documentation and other small bugs you had raised.

@moghadas76 not stealing anyone's work here. Just want to get this PR merged so that I can start using it in my repo without doing weird installation. I have not yet raised a PR, and can wait if you have time to get this done.

stillmatic · 2024-07-01T21:04:18Z

Happy to make my suggestions as comments on this branch if you have the time to address them here. I appreciate the work - I used the implementation here in my training, but ran into some problems, hence seeing what needs improvement.

kallewoof · 2024-07-02T05:47:04Z

This is very disrespectful. He stole this branch.

Assuming proper credit is given, there's nothing disrespectful about picking up someone's work if they are unable to complete it in a timely manner; there are month+ gaps between you receiving review and you actually addressing it. The PR is 6 4 months old, for no particular reason at all. Let's get this polished up and merged.

kallewoof · 2024-07-02T06:24:22Z

I have rebased and done some fixes on top of this pull request here: https://github.com/kallewoof/peft/tree/202407-loraplus

@moghadas76 You can either base your work off of my fixes or redo it yourself. Whatever gets this merged the fastest. I specifically did not make a pull request out of this, as it sounds like you really want to do this yourself.

@stillmatic Did I get your suggestions in there correctly?

BenjaminBossan · 2024-07-02T09:07:06Z

I agree that this is not about "stealing" work. All of this is a big collaboration, after all the initial PR was heavily based on https://github.com/nikhil-ghosh-berkeley/loraplus/blob/main/lora_plus.py.

It would be best if we can get this PR over the finish line, as we're not missing a lot. @moghadas76 if you are still interested, let's try to finish this in the next two weeks. There has been some good feedback in this thread, so I'm sure we have everything we need to get this ready.

If there is no progress here, I'm happy to merge other PRs that implement the same idea, with proper references being given. I'll ensure that co-authorship is respected when merging.

stillmatic · 2024-07-02T13:08:26Z

src/peft/optimizers/loraplus.py

+    ]
+
+    optimizer = optimizer_cls(optimizer_grouped_parameters, **optimizer_kwargs)
+    if optimizer_cls.__name__ == "Adam8bit":


should this support the other 8-bit adam implementations?

@kallewoof this is the only important comment I had, simplest would be

eight_bit_names = ["Adam8bit", "AdamW8bit", "PagedAdam8bit", "PagedAdamW8bit"] if optimizer_cls.__name__ in eight_bit_names:

Thanks. Added to proposed branch.

stillmatic · 2024-07-02T13:12:37Z

src/peft/optimizers/loraplus.py

+    """
+    from ..tuners.lora.layer import Embedding
+
+    loraplus_lr_ratio = optimizer_kwargs.pop("loraplus_lr_ratio")


it's confusing to me that loraplus_lr_ratio is an optimizer_kwarg while loraplus_lr_embedding is a function argument. IMO both should be function arguments, while optimizer_kwarg should reflect the arguments passed to the optimizer.

I made both of them args, outside of optimizer_kwarg in https://github.com/kallewoof/peft/tree/202407-loraplus FWIW, based on your comment.

stillmatic · 2024-07-02T13:13:57Z

src/peft/optimizers/loraplus.py

+    Args:
+        model (`torch.nn.Module`): The model to be optimized.
+        optimizer_cls (`torch.optim.Optimizer`): The optimizer class to be used.
+        optimizer_kwargs (`dict`): Additional keyword arguments to be passed to the optimizer.


should note explicitly that lr and weight_decay are expected.

Addressed in the proposed changes in https://github.com/kallewoof/peft/tree/202407-loraplus

kallewoof · 2024-07-08T14:43:26Z

It's been a week, so I opened the above branch as a pull req. Close it if that's not OK, @BenjaminBossan.

BenjaminBossan · 2024-07-08T15:37:21Z

Thanks @kallewoof. Starting tomorrow until the end of the week, I'll be at EuroPython Prague, so I will have little time for reviews etc. If by then, there is no progress on this PR, we can close it and continue with yours. As mentioned earlier, I'll make sure to assign proper credit before merging.

BenjaminBossan · 2024-07-29T10:51:02Z

Supersedes by #1915.

Add lora+ implentation

972fa75

BenjaminBossan closed this Feb 26, 2024

BenjaminBossan reopened this Feb 26, 2024

BenjaminBossan mentioned this pull request Feb 29, 2024

Feature Request: Integrate Lora+/different learning rates for adapter matrices A and B #1504

Closed

Support LoraPlus cfg

f95ee34

BenjaminBossan requested changes Mar 18, 2024

View reviewed changes

moghadas76 added 2 commits March 31, 2024 01:31

Fix QA comments

0968391

Fix test

fb8d954

moghadas76 added 2 commits April 19, 2024 21:54

Fix tests

f148f0e

Merge branch 'main' into feature/loraplus

888d2f1

Merge branch 'main' into feature/loraplus

e86f8b6

BenjaminBossan requested changes Apr 26, 2024

View reviewed changes

src/peft/utils/peft_types.py Outdated Show resolved Hide resolved

src/peft/helpers.py Outdated Show resolved Hide resolved

src/peft/helpers.py Outdated Show resolved Hide resolved

tests/test_loraplus_helper.py Show resolved Hide resolved

Fix styling problem

f4c4e58

BenjaminBossan requested changes May 22, 2024

View reviewed changes

tests/test_loraplus_helper.py Show resolved Hide resolved

tests/test_loraplus_helper.py Show resolved Hide resolved

Fix formatter

5d0b1b7

BenjaminBossan mentioned this pull request May 27, 2024

Add Special Optimizer for LoRA training #1803

Closed

fangzhaozhang mentioned this pull request May 28, 2024

Integrating Riemannian Preconditioner #1807

Closed

moghadas76 added 2 commits June 9, 2024 18:12

Fix QA comments

519cd41

Merge branch 'main' into feature/loraplus

6e72a09

Fix docs

8353825

stillmatic reviewed Jul 2, 2024

View reviewed changes

kallewoof mentioned this pull request Jul 8, 2024

Add lora+ implementation #1915

Merged

BenjaminBossan closed this Jul 29, 2024

Add lora+ implentation #1509

Add lora+ implentation #1509

Conversation

moghadas76 commented Feb 26, 2024

BenjaminBossan commented Feb 26, 2024 • edited Loading

HuggingFaceDocBuilderDev commented Feb 26, 2024

moghadas76 commented Feb 26, 2024

BenjaminBossan commented Feb 26, 2024

moghadas76 commented Feb 26, 2024

BenjaminBossan commented Mar 6, 2024

BenjaminBossan commented Mar 12, 2024

moghadas76 commented Mar 12, 2024 via email

BenjaminBossan commented Mar 12, 2024

moghadas76 commented Mar 17, 2024

BenjaminBossan left a comment • edited Loading

Choose a reason for hiding this comment

BenjaminBossan commented Mar 25, 2024

moghadas76 commented Mar 25, 2024 via email

BenjaminBossan commented Mar 25, 2024

BenjaminBossan commented Apr 2, 2024

BenjaminBossan commented Apr 19, 2024

moghadas76 commented Apr 19, 2024

moghadas76 commented Apr 19, 2024

BenjaminBossan commented Apr 25, 2024

BenjaminBossan left a comment

Choose a reason for hiding this comment

BenjaminBossan commented May 21, 2024

moghadas76 commented May 21, 2024

BenjaminBossan left a comment

Choose a reason for hiding this comment

moghadas76 commented Jun 9, 2024

BenjaminBossan commented Jun 10, 2024

shubhamjain0594 commented Jun 23, 2024

BenjaminBossan commented Jun 24, 2024

shubhamjain0594 commented Jun 24, 2024 • edited Loading

BenjaminBossan commented Jun 24, 2024

stillmatic commented Jul 1, 2024

BenjaminBossan commented Jul 1, 2024

moghadas76 commented Jul 1, 2024

shubhamjain0594 commented Jul 1, 2024 • edited Loading

stillmatic commented Jul 1, 2024

kallewoof commented Jul 2, 2024 • edited Loading

kallewoof commented Jul 2, 2024

BenjaminBossan commented Jul 2, 2024

stillmatic Jul 2, 2024

Choose a reason for hiding this comment

stillmatic Jul 2, 2024

Choose a reason for hiding this comment

kallewoof Jul 2, 2024

Choose a reason for hiding this comment

stillmatic Jul 2, 2024

Choose a reason for hiding this comment

kallewoof Jul 2, 2024

Choose a reason for hiding this comment

stillmatic Jul 2, 2024

Choose a reason for hiding this comment

kallewoof Jul 2, 2024

Choose a reason for hiding this comment

kallewoof commented Jul 8, 2024

BenjaminBossan commented Jul 8, 2024

BenjaminBossan commented Jul 29, 2024

BenjaminBossan commented Feb 26, 2024 •

edited

Loading

BenjaminBossan left a comment •

edited

Loading

shubhamjain0594 commented Jun 24, 2024 •

edited

Loading

shubhamjain0594 commented Jul 1, 2024 •

edited

Loading

kallewoof commented Jul 2, 2024 •

edited

Loading