Add new merging methods #1364

pacman100 · 2024-01-16T13:50:34Z

What does this PR do?

Add new model merging methods for LoRA based on the papers TIES-MERGING: Resolving Interference When Merging Models and Language Models are Super Mario: Absorbing Abilities from Homologous Models as a Free Lunch. The methods are ties, dare_linear, dare_ties, ties_svd, dare_linear_svd, dare_ties_svd.
The inspiration for the implementation of these methods are from https://github.com/yule-BUAA/MergeLM/tree/main and https://github.com/cg123/mergekit/tree/main.

Example of ties_svd is shown below (https://github.com/pacman100/peft-dreambooth-ui/blob/main/lora_merging.ipynb):

LLM LoRA merging example:

To do:

Add tests
Add documentation
Add example notebook

HuggingFaceDocBuilderDev · 2024-01-16T13:56:22Z

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.

review-notebook-app · 2024-01-17T09:23:37Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

younesbelkada

Thanks ! I left few open questions - what do you think?

younesbelkada · 2024-01-17T16:07:01Z

src/peft/tuners/lora/model.py

        for adapter, weight in zip(adapters, weights):
            if adapter in target.lora_A or adapter in target.lora_embedding_A:
                valid_adapters.append(adapter)
-                valid_weights.append(weight)
+                valid_weights.append(weight * target.scaling[adapter])


is this change somehow breaking?

Hello Younes, this should be the correct usage, earlier the implementation was incorrect as it was missing scaling factor.

src/peft/utils/merge_utils.py

sayakpaul

Left some nits.

I really like how you have broken down the core merging methods in terms of small logical components that are shared across.

I think it definitely makes sense to also add a detailed guide about these methods on the PEFT doc site. But this can come later. Cc: @MKhalusova @stevhliu

stevhliu

Really nice!

Do you think it'd also be nice to create a separate API reference page for these merging utilities so it's easy for users to find? I can work on this in a separate PR in addition to the guide suggested by @sayakpaul :)

src/peft/tuners/lora/model.py

src/peft/utils/merge_utils.py

prateeky2806 · 2024-01-19T17:25:07Z

Hi @pacman100 @younesbelkada @stevhliu @sayakpaul, I am Prateek Yadav, the author of TIES-Merging. Let me know if there is some way I can contribute to adding these merging methods to the Transformers or the PEFT library.

Also, I agree with others here that having some sort of changes to the documentation or updates to the README can help the users to be aware of these merging methods, and how to use them.

Let me know if there is some for me help!

Thanks,
Prateek

Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>

prateeky2806 · 2024-02-03T05:47:03Z

Hi @pacman100, I went over the code for TIES and it seems good to me. I just mentioning the embedding layer thing might be important at least for merging full models.

prateeky2806 · 2024-02-03T05:49:32Z

Let's go 🚀

How are we planning on documenting this as best as we can?

Hi @sayakpaul, I guess this is the pull request trying to document these merging methods

sayakpaul · 2024-02-03T05:51:17Z

Ah, you're correct. Thanks for the mention.

prateeky2806 · 2024-02-04T19:17:35Z

Hi @sayakpaul @pacman100, is there a timeline on when this is expected to be merged?

sayakpaul · 2024-02-05T02:17:28Z

I think we can merge now. @younesbelkada WDYT?

BenjaminBossan

Thanks a lot @pacman100 for this excellent PR. Super well done and clean implementation + refactor!

In my review, I didn't check the correctness of the implementations in detail, since the original authors already did that and are much more qualified for this. Instead, I focused on the other parts, please take a look. Most things should be fairly easy adjustments.

Regarding the new functions in merge_utils.py, I think it will be good to add some unit tests for those in a future PR.

src/peft/tuners/lora/model.py

BenjaminBossan · 2024-02-06T11:11:27Z

src/peft/tuners/lora/model.py

            new_rank = adapters_ranks[0]
        elif combination_type == "cat":
            # adapters ranks may be different, new rank is sum of all ranks
            # be careful, because output adapter rank may be really big if mixing a lot of adapters
            new_rank = sum(adapters_ranks)
-        elif combination_type == "svd":
+        elif "svd" in combination_type:


I would prefer:

Suggested change

elif "svd" in combination_type:

elif (combination_type == "svd") or (combination_type == "ties_svd"):

This is to prevent potential bugs in the future where we add a new combination type with a strange name that has "svd" as a substring.

but it is also meant for dare_linear_svd, dare_ties_svd too

Oh I see. So maybe let's check all these options explicitly, or use if combination_type.endswith("svd"), which should be less error prone.

src/peft/tuners/lora/model.py

src/peft/utils/merge_utils.py

BenjaminBossan · 2024-02-06T11:28:16Z

src/peft/utils/merge_utils.py

+    return sign == majority_sign
+
+
+def disjoint_merge(task_tensors: torch.Tensor, majority_sign_mask: torch.Tensor) -> torch.Tensor:


task_tensors is not a list of tensors but a tensor, right? Below, task_tensors is always a list of tensors. I wonder if it would make sense to use a different variable name for this function to avoid confusion.

src/peft/utils/merge_utils.py

tests/testing_common.py

src/peft/utils/merge_utils.py

prateeky2806 · 2024-02-07T00:28:51Z

Hi @pacman100 and @BenjaminBossan, I went into detail over the merge_utils file and left some comments, I feel like some parts need to be corrected and might have bugs if I am not missing something.

I think I accidentally left many comments as opposed to reviewing but I guess that should be fine.

Co-Authored-By: Prateek Yadav <15224633+prateeky2806@users.noreply.github.com> Co-Authored-By: Yu Le <55241218+yule-buaa@users.noreply.github.com> Co-Authored-By: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

BenjaminBossan

Thanks for addressing the issues from my side. This LGTM now. One small comment left, but it's not a must.

BenjaminBossan · 2024-02-08T10:44:49Z

src/peft/tuners/lora/model.py

            new_rank = adapters_ranks[0]
        elif combination_type == "cat":
            # adapters ranks may be different, new rank is sum of all ranks
            # be careful, because output adapter rank may be really big if mixing a lot of adapters
            new_rank = sum(adapters_ranks)
-        elif combination_type == "svd":
+        elif "svd" in combination_type:


Oh I see. So maybe let's check all these options explicitly, or use if combination_type.endswith("svd"), which should be less error prone.

prateeky2806 · 2024-02-12T01:51:08Z

Hi @pacman100, Thanks for working on this and merging this pull request. As we discussed earlier, I was wondering if this would be announced as a part of the next PEFT version or if should I just announce it on my Twitter right away. I feel like it would be much better to announce it with the next PEFT version, however, I am not sure when that would be. Moreover, the PR on adding the docs is still ongoing.

Thanks,
Prateek

sayakpaul · 2024-02-12T02:54:05Z

Hey Prateek!

@pacman100 and I are working on a blog post to discuss this which we plan to release very soon. We will of course give everyone the due credits there. I think it makes sense to also do a release for this feature so that users don't have to install peft from the main.

Does that work for you?

prateeky2806 · 2024-02-12T04:11:17Z

Hi @sayakpaul, yes that sounds good to me looking forward to it. I will tweet about this once you do this release. Moreover, if you someone to proof read the blog post then I would be happy to help you guys with it.

Prateek

yule-BUAA · 2024-02-12T14:27:19Z

Hi @pacman100 and @sayakpaul,
Thanks for merging this PR into the main branch of peft. I would also be glad to do proofreading for the doc or blog post on merging models if there are any things in need.

Le Yu

sayakpaul · 2024-02-12T14:40:32Z

Appreciate all the support. We will keep you posted.

prateeky2806 · 2024-02-13T05:54:44Z

src/peft/utils/merge_utils.py

+
+def ties(
+    task_tensors: List[torch.Tensor],
+    weights: torch.Tensor,


Hi @pacman100 @sayakpaul, I have just one last suggestion. It would be ideal to set the default weight for TIES to all ones because that's a setting we experimented with in our paper and kind of works well if people do not want to try out different values. You can look at the results with the red cross in this table, they are with default weights of all ones. This makes TIES as simple to use as basic averaging by avoiding the necessity to select the weights.

So by default TIES should have 20% of the parameters remaining and mixing weight = 1 for each peft module.

* add code * update docstring * quality * fix test * fix test * fix svd embedding layer merging * fixes * fixes * Update model.py * Add test and example * quality * fix tests * update the example * Apply suggestions from code review Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com> * address comments * address comments and add co-authors Co-Authored-By: Prateek Yadav <15224633+prateeky2806@users.noreply.github.com> Co-Authored-By: Yu Le <55241218+yule-buaa@users.noreply.github.com> Co-Authored-By: Benjamin Bossan <BenjaminBossan@users.noreply.github.com> * quality * Update merge_utils.py * revert * address comments * address comment --------- Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com> Co-authored-by: Prateek Yadav <15224633+prateeky2806@users.noreply.github.com> Co-authored-by: Yu Le <55241218+yule-buaa@users.noreply.github.com> Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

pacman100 added 3 commits January 16, 2024 18:57

add code

8f49390

update docstring

c08be3b

quality

689a4d9

pacman100 added 7 commits January 16, 2024 19:57

fix test

e47331d

fix test

be8e39d

fix svd embedding layer merging

3e42473

fixes

f6fda4b

fixes

82fe281

Update model.py

42af5b8

Add test and example

91890ea

pacman100 added 3 commits January 17, 2024 14:55

quality

ece6309

fix tests

904b794

update the example

83f2538

pacman100 requested review from younesbelkada and sayakpaul January 17, 2024 13:37

younesbelkada approved these changes Jan 17, 2024

View reviewed changes

sayakpaul reviewed Jan 18, 2024

View reviewed changes

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

sayakpaul reviewed Jan 18, 2024

View reviewed changes

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

sayakpaul reviewed Jan 18, 2024

View reviewed changes

src/peft/utils/merge_utils.py Show resolved Hide resolved

sayakpaul reviewed Jan 18, 2024

View reviewed changes

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

sayakpaul reviewed Jan 18, 2024

View reviewed changes

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

sayakpaul reviewed Jan 18, 2024

View reviewed changes

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

sayakpaul approved these changes Jan 18, 2024

View reviewed changes

stevhliu approved these changes Jan 18, 2024

View reviewed changes

src/peft/tuners/lora/model.py Outdated Show resolved Hide resolved

src/peft/tuners/lora/model.py Outdated Show resolved Hide resolved

src/peft/tuners/lora/model.py Outdated Show resolved Hide resolved

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

tgaddair mentioned this pull request Jan 30, 2024

Merge multiple LoRA adapters per request (linear, TIES, DARE) predibase/lorax#212

Merged

Apply suggestions from code review

a4bf0fb

Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>

stevhliu mentioned this pull request Jan 31, 2024

[docs] Model merging #1423

Merged

2 tasks

pacman100 requested a review from BenjaminBossan February 5, 2024 10:12

BenjaminBossan requested changes Feb 6, 2024

View reviewed changes

prateeky2806 reviewed Feb 6, 2024

View reviewed changes

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

prateeky2806 reviewed Feb 7, 2024

View reviewed changes

src/peft/utils/merge_utils.py Show resolved Hide resolved

prateeky2806 reviewed Feb 7, 2024

View reviewed changes

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

sayakpaul mentioned this pull request Feb 7, 2024

[Discussion] allow new merging methods from peft huggingface/diffusers#6892

Closed

pacman100 and others added 6 commits February 8, 2024 12:28

address comments and add co-authors

4205b41

Co-Authored-By: Prateek Yadav <15224633+prateeky2806@users.noreply.github.com> Co-Authored-By: Yu Le <55241218+yule-buaa@users.noreply.github.com> Co-Authored-By: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

Merge branch 'main' into smangrul/add-new-merging-methods

22d5434

quality

7418713

Update merge_utils.py

a63d003

revert

261fc9c

address comments

b6f3012

BenjaminBossan approved these changes Feb 8, 2024

View reviewed changes

address comment

69001b6

pacman100 merged commit c1a83fd into main Feb 9, 2024
14 checks passed

sayakpaul deleted the smangrul/add-new-merging-methods branch February 12, 2024 02:52

prateeky2806 reviewed Feb 13, 2024

View reviewed changes

sayakpaul mentioned this pull request Feb 16, 2024

add peft merging blog post. huggingface/blog#1816

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new merging methods #1364

Add new merging methods #1364

pacman100 commented Jan 16, 2024 •

edited

Loading

HuggingFaceDocBuilderDev commented Jan 16, 2024

review-notebook-app bot commented Jan 17, 2024

younesbelkada left a comment

younesbelkada Jan 17, 2024

pacman100 Jan 31, 2024

sayakpaul left a comment

stevhliu left a comment

prateeky2806 commented Jan 19, 2024

prateeky2806 commented Feb 3, 2024

prateeky2806 commented Feb 3, 2024

sayakpaul commented Feb 3, 2024

prateeky2806 commented Feb 4, 2024

sayakpaul commented Feb 5, 2024

BenjaminBossan left a comment

BenjaminBossan Feb 6, 2024

pacman100 Feb 8, 2024

BenjaminBossan Feb 8, 2024

BenjaminBossan Feb 6, 2024

prateeky2806 commented Feb 7, 2024

BenjaminBossan left a comment

BenjaminBossan Feb 8, 2024

prateeky2806 commented Feb 12, 2024

sayakpaul commented Feb 12, 2024

prateeky2806 commented Feb 12, 2024 •

edited

Loading

yule-BUAA commented Feb 12, 2024

sayakpaul commented Feb 12, 2024

prateeky2806 Feb 13, 2024

prateeky2806 Feb 15, 2024

	elif "svd" in combination_type:
	elif (combination_type == "svd") or (combination_type == "ties_svd"):

		return sign == majority_sign


		def disjoint_merge(task_tensors: torch.Tensor, majority_sign_mask: torch.Tensor) -> torch.Tensor:

Add new merging methods #1364

Add new merging methods #1364

Conversation

pacman100 commented Jan 16, 2024 • edited Loading

What does this PR do?

HuggingFaceDocBuilderDev commented Jan 16, 2024

review-notebook-app bot commented Jan 17, 2024

younesbelkada left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sayakpaul left a comment

Choose a reason for hiding this comment

stevhliu left a comment

Choose a reason for hiding this comment

prateeky2806 commented Jan 19, 2024

prateeky2806 commented Feb 3, 2024

prateeky2806 commented Feb 3, 2024

sayakpaul commented Feb 3, 2024

prateeky2806 commented Feb 4, 2024

sayakpaul commented Feb 5, 2024

BenjaminBossan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prateeky2806 commented Feb 7, 2024

BenjaminBossan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prateeky2806 commented Feb 12, 2024

sayakpaul commented Feb 12, 2024

prateeky2806 commented Feb 12, 2024 • edited Loading

yule-BUAA commented Feb 12, 2024

sayakpaul commented Feb 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pacman100 commented Jan 16, 2024 •

edited

Loading

prateeky2806 commented Feb 12, 2024 •

edited

Loading