ENH: Tie weights for target_modules in Lora (#2864) #2879

romitjain · 2025-10-29T06:01:11Z

Solves #2864 for target_modules

Enables ensure_weight_tying flag in LoraConfig for target_modules.

For LoRA, if any of the tied layers are added to target_modules and ensure_weight_tying == True, the adapters added to the layer are shared with all the tied layers.

For example, if a model has tied weights and target_modules=['embed_tokens'] then, LoRA adapters are added to both embed_tokens and lm_head. The adapters in lm_head share the weights with the adapters added to embed_tokens

romitjain · 2025-10-29T06:02:58Z

@BenjaminBossan
I have added the relevant test cases and implemented the ensure_weight_tying flag for target_modules. The current implementation works only if embed_tokens is added and not if lm_head is added. I will implement that fix and update the PR, but meanwhile would appreciate your views on the logic and implementation.

At a high level

I have updated BaseTuner._check_tied_modules to check for tied modules in target_modules
I have added a private method BaseTuner._add_targets_to_tie that needs to be implemented by the inheriting classes
I have added a loop in BaseTuner.inject_adapter to tie the adapters. I have implemented this extra loop to ensure that the order in which adapters are added to the target modules do not matter.

Thank you

BenjaminBossan

Thanks for this draft PR to extend the feature to target_modules. I haven't done a full review yet, as some implementation details have yet to be figured out, but I gave some early feedback. This feature could be a bit more difficult to implement than for modules_to_save, I added some comments on why, please check.

src/peft/tuners/lora/config.py

BenjaminBossan · 2025-10-29T10:54:22Z

src/peft/tuners/lora/model.py

        peft_config.modules_to_tie = missing_keys
+
+    def _add_targets_to_tie(self, peft_config, tied_weight_keys):
+        target_modules = set(getattr(peft_config, "target_modules", []) or [])


We need to consider the case that target_modules is a string and not a list of strings. If it's a string, we perform a regex match. Honestly, I'm not sure if there is a good solution. So far, I have 3 ideas:

We could try to use the model.targeted_module_names attribute, which lists all targeted modules after the targets have been resolved. But that would mean that we need to first apply all LoRA layers and only then can we check for tied layers, which is the opposite order of how things are implemented right now.

We could try using the string directly and then for example do something like: config.target_modules += f"|{missing_key}" but this is very brittle and won't work with all regexes, so I would like to avoid this.

We could forbid using ensure_weight_tying=True and target_modules = <str>. Then we'd raise an error and tell users they have to pass a list of str if they want ensure_weight_tying.

Yes, after going through the code a few more times, I realized this would not work for all the cases. I would go with the 1st approach and move the call to this function after model.targeted_module_names is updated

Sounds good. This has yet to be updated, right?

Yes, I will do this in the next commit

@BenjaminBossan

Moving this after model.targeted_modules_name is populated is tough, as the loop which populates this (https://github.com/huggingface/peft/blob/main/src/peft/tuners/tuners_utils.py#L773-L819) also needs to check and skip if the layers are tied.

Reversing the order would mean that we may end up adding adapters where they're not required. The subsequent code would become more involved, but essentially, we would have to remove adapters from all tied layers, re-add in embed_tokens, and proceed to tie remaining adapters to this. This is an opinionated solve which has the least complexity, according to me.

We can go with (1) in your original comment and redo a few things, or keep the current flow and go with (3).

I think the above might have become tough to follow 😅, so let me know and I can share some schematics. Will wait for your input.

src/peft/tuners/tuners_utils.py

Signed-off-by: romit <romit@ibm.com>

src/peft/tuners/tuners_utils.py

romitjain · 2025-10-31T12:14:51Z

@BenjaminBossan This is now ready for review. I have also updated the logic for tied layers in modules_to_save so that lm_head and [embed_tokens, lm_head] cases are supported. Earlier, they would not have worked. The high level implementation remains the same but according to me it's much better placed then my earlier commits.

I have also added a few tests for the above case, and all of the tests pass.

The only thing remaining is how to check for target_modules in case it's a string. I will come back to it, but you can go ahead and review the core logic.

BenjaminBossan

Thanks for the latest updates. This looks much cleaner now, I think we're approaching the finish line. There were still some issues I had though, so please check my comments.

src/peft/tuners/lora/layer.py

src/peft/tuners/lora/model.py

src/peft/tuners/tuners_utils.py

tests/test_initialization.py

Signed-off-by: romit <romit@ibm.com>

Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

Signed-off-by: romit <romit@ibm.com>

romitjain · 2025-11-05T12:20:50Z

@BenjaminBossan I have addressed your comments. PTAL

BenjaminBossan

Thanks for the updates. I think there are some yet unaddressed comments from before and I also added a few more, please check.

There is, however, a bit of a blocker right now. Currently, a huge PR in transformers is on the way: huggingface/transformers#41580. It is intended to be released soon with transformers v5. A change that might affect us is that _tied_weights_keys will be converted from a list to a dict (with keys being targets and values sources). It could also affect _get_tied_weight_keys. We're still discussing how this will affect PEFT. Possibly it's going to be fine, but we're not sure yet, the PR is still changing.

src/peft/peft_model.py

src/peft/tuners/lora/model.py

BenjaminBossan · 2025-11-06T11:43:56Z

src/peft/tuners/lora/model.py

        peft_config.modules_to_tie = missing_keys
+
+    def _add_targets_to_tie(self, peft_config, tied_weight_keys):
+        target_modules = set(getattr(peft_config, "target_modules", []) or [])


Sounds good. This has yet to be updated, right?

src/peft/tuners/lora/model.py

Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

…to enh/tie-target-modules

Signed-off-by: romit <romit@ibm.com>

romitjain · 2025-11-07T13:29:20Z

src/peft/tuners/lora/model.py

+        tied_weight_keys = set(tied_weight_keys)
+        peft_config.target_modules_to_tie = tied_weight_keys
+
+        raw_target_modules = getattr(peft_config, "target_modules", None)


@BenjaminBossan Please review this logic. I know this is a bit hacky! I am open to suggestions

Hmm yeah, this is rough. We can't really operate on the string like this, as there are too many possible ways that the regex could be formed. I wonder if we should just leave it be and deal with the tied module edge case in inject_adapter directly. I haven't fully thought this through, perhaps you already tried that and there is a caveat that I'm missing?

#2879 (comment)

It should be possible, it would just make the flow very convoluted.

I redid this a bit. We just need to make sure that embed_tokens is present in the target_modules

tests/test_initialization.py

romitjain · 2025-11-07T13:34:35Z

@BenjaminBossan please review the latest changes now. I believe I have addressed all your comments, but let me know if I missed something.

I have added test cases where we are passing target_modules as str and added a (slightly) hacky solve for that. It's opinionated to keep the flow simple.

Regarding the transformers v5 update, since we would be having a version locked in peft, I believe if this PR advances faster than that, we can merge this. I can take up changes too whenever they're needed. However, you are much closer to this, so you can decide and let me know.

BenjaminBossan

Thanks for the new updates. I added my comments, as usual :)

Regarding the transformers v5 update, since we would be having a version locked in peft, I believe if this PR advances faster than that, we can merge this. I can take up changes too whenever they're needed. However, you are much closer to this, so you can decide and let me know.

No, we don't have a version lock, our goal is to ensure that the upcoming PEFT release v0.18.0 is compatible with both transformers v5 and older versions of transformers. As we have a feature freeze for PEFT, it means that this PR will have to wait for after the 0.18.0 release. When it is ready to merge, we should hopefully know the final state of transformers v5 and then we can test the PR with it to ensure the tests still pass.

src/peft/peft_model.py

BenjaminBossan · 2025-11-10T13:57:45Z

src/peft/tuners/lora/model.py

+        peft_config.modules_to_tie = tied_weight_keys
+
+        modules_to_save = getattr(peft_config, "modules_to_save", []) or []
+        if "embed_tokens" not in modules_to_save:


If the embedding layer has a different name, this won't be correct, right? It's probably still fine for now.

Yes, that is true. I am not able to find a way to get the embedding layer name from the model

We could try to find the layer name whose parameter corresponds to model.get_input_embedding() but I'm fine with assuming the name here. Let's just add a comment.

BenjaminBossan · 2025-11-10T14:05:37Z

src/peft/tuners/tuners_utils.py

        if getattr(peft_config, "ensure_weight_tying", False):
-            if is_embedding_to_save and tied_weight_keys:
-                self._add_modules_to_tie(peft_config, tied_weight_keys)
+            if (is_embedding_to_save or is_embedding_in_target) and tied_weight_keys:


I think this whole block, line 1288-1298, can be replaced with:

if getattr(peft_config, "ensure_weight_tying", False): if tied_weight_keys: if is_embedding_to_save: self._add_modules_to_tie(peft_config, tied_weight_keys) elif is_embedding_in_target: self._add_targets_to_tie(peft_config, tied_weight_keys) else: warnings.warn( "You have requested `ensure_weight_tying`, but no tied modules are added in either " "`modules_to_save` or `target_modules`" ) else: warnings.warn("You have requested `ensure_weight_tying`, but no tied modules were found in the model")

I think this is cleaner.

src/peft/tuners/tuners_utils.py

BenjaminBossan · 2025-11-10T14:23:56Z

src/peft/tuners/lora/model.py

+        tied_weight_keys = set(tied_weight_keys)
+        peft_config.target_modules_to_tie = tied_weight_keys
+
+        raw_target_modules = getattr(peft_config, "target_modules", None)


Hmm yeah, this is rough. We can't really operate on the string like this, as there are too many possible ways that the regex could be formed. I wonder if we should just leave it be and deal with the tied module edge case in inject_adapter directly. I haven't fully thought this through, perhaps you already tried that and there is a caveat that I'm missing?

Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

Signed-off-by: romit <romit@ibm.com>

romitjain · 2025-11-12T07:05:26Z

@BenjaminBossan I have made the updates. As of now, we have 3 outstanding updates we need to resolve

What to do in case target_modules is a str: ENH: Tie weights for target_modules in Lora (#2864) #2879 (comment): I have replied to the comment with a small modification in the logic
Release of this PR: Can this PR be merged to main but not in release? Or do we have an ETA for v5 arrival? Some of our internal PRs depend on this, hence asking.
(Minor) Duplication of a warning message: ENH: Tie weights for target_modules in Lora (#2864) #2879 (comment) I have replied to the comment

BenjaminBossan

Thanks for the updates. I did only a light pass, since we're still discussing implementation details.

Moving this after model.targeted_modules_name is populated is tough, as the loop which populates this (https://github.com/huggingface/peft/blob/main/src/peft/tuners/tuners_utils.py#L773-L819) also needs to check and skip if the layers are tied.

Reversing the order would mean that we may end up adding adapters where they're not required. The subsequent code would become more involved, but essentially, we would have to remove adapters from all tied layers, re-add in embed_tokens, and proceed to tie remaining adapters to this. This is an opinionated solve which has the least complexity, according to me.

We can go with (1) in your original comment and redo a few things, or keep the current flow and go with (3).

I think the above might have become tough to follow 😅, so let me know and I can share some schematics. Will wait for your input.

So IIUC, we already have code for skipping the tied layers, so that's already taken into account, right? For the rest, maybe you could share high level pseudo code so that I can get an idea?

Adding layers whose weights will be overridden later is btw. not something I'm overly concerned with. The number of tied weights is usually quite small in the grand scheme of things, so the overhead should be negligible.

Release of this PR: Can this PR be merged to main but not in release? Or do we have an ETA for v5 arrival? Some of our internal PRs depend on this, hence asking.

So we decided to go ahead with the PEFT release, as it's unclear when v5 will come. We merged #2902, which should take care of forward compatibility (but it causes the merge conflict now, please take care of it). This means that once this PR is ready, we can merge it starting tomorrow.

src/peft/peft_model.py

BenjaminBossan · 2025-11-12T16:17:02Z

src/peft/tuners/lora/model.py

+        peft_config.modules_to_tie = tied_weight_keys
+
+        modules_to_save = getattr(peft_config, "modules_to_save", []) or []
+        if "embed_tokens" not in modules_to_save:


We could try to find the layer name whose parameter corresponds to model.get_input_embedding() but I'm fine with assuming the name here. Let's just add a comment.

src/peft/tuners/lora/model.py

src/peft/tuners/tuners_utils.py

Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

Signed-off-by: romit <romit@ibm.com>

…modules

romitjain · 2025-11-13T08:47:31Z

ENH: Tie weights for target_modules in Lora (#2864) #2879 (comment): Good suggestion. I have added a new function to find a layer by reference tensors
I have resolved the merge conflict and addressed your remaining comments
Here's a psudo-code of the complete flow that is currently implemented

def inject_adapter():
    ...

    # finds tied modules and adds to a set - target_modules_to_tie
    self._check_tied_modules(model, peft_config)

    tied_targets = []

    # loop 1
    for k in keys:
        result = k in target_modules
        to_tie = k in target_modules_to_tie

        if to_tie:
            tied_targets.append(k)
            continue
        if result:
            add_lora(k)

    # loop 2
    for t in tied_targets:
        add_tied_lora(t)

    ...

Here is the flow of the alternate that you suggested

def inject_adapter():
    ...
    # loop 1
    for k in keys:
        result = k in target_modules
        if result:
            add_lora(k)


    # finds tied modules and adds to a set - target_modules_to_tie
    self._check_tied_modules(model, peft_config)

    # this is needed since all the tied adapters reference embedding
    # layer's lora as source
    add_lora(embed_tokens)

    # loop 2
    for t in target_modules_to_tie:
        remove_lora(t) # will remove lora if it exists, this might be a bit complex
        add_tied_lora(t)

    ...

HuggingFaceDocBuilderDev · 2025-11-14T11:06:31Z

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.

BenjaminBossan

Thanks for the latest improvements to the PR. As you can see, there is one test failing because of a warning not matching, could you please take a look?

Here's a psudo-code of the complete flow that is currently implemented

Thanks for this. I think the second approach wouldn't actually differ that much when it comes to complexity, as such I'm fine with keeping the current approach.

Signed-off-by: romit <romit@ibm.com>

romitjain · 2025-11-14T12:15:32Z

@BenjaminBossan, I have fixed the test. I have removed 3 redundant tests which are no longer required.

BenjaminBossan

Thanks for your continued work. From my perspective, this LGTM (just a small nit).

Overall, this all turned out to be more complex than I would have hoped, but I'm unsure if there is a better way. @githubnemo it would be great if you could also review, maybe you have some ideas.

tests/test_initialization.py

Signed-off-by: romit <romit@ibm.com>

romitjain · 2025-11-18T16:59:50Z

@BenjaminBossan Resolved your comment

Signed-off-by: romit <romit@ibm.com>

romitjain · 2025-11-19T14:22:00Z

@BenjaminBossan I made a small commit - in one of my earlier commits, I had made a change where the target modules were saved as model.embed_tokens instead of embed_tokens. This was causing some downstream issues.
Apologies for the late commit, I know that you will have to trigger the runs again 😅

BenjaminBossan · 2025-11-20T16:12:02Z

Thanks for the update @romitjain. There is a new merge conflict, could you please check?

…t-modules

romitjain · 2025-11-20T16:59:09Z

@BenjaminBossan Done

romitjain · 2025-11-21T11:51:16Z

@BenjaminBossan Let me know if any steps are remaining from my side for final push?

BenjaminBossan · 2025-11-21T14:44:22Z

@romitjain No, thank you, let's wait for @githubnemo's review.

romitjain · 2025-11-28T14:59:31Z

Hi @githubnemo, it would be very helpful if you could review the PR. One of our internal features depends on this :)

romitjain · 2025-12-03T06:29:35Z

Hi @githubnemo, gentle reminder. Would really appreciate an update. Thanks

githubnemo

Hey, sorry for the delayed review.

Thanks for working on this, it's quite the gnarly topic :)

I'm not sure if I understood everything correctly so some of my comments may just be me misunderstanding something but I think that there are some places where the match is rather probabilistic.

I wonder if we need to normalize layer names at some point so that we only work with fully-qualified names after that point. For example in _add_modules_to_tie we will look at the modules to save set:

modules_to_save = getattr(peft_config, "modules_to_save", []) or []

I don't think that we have guaranteed fully-qualified names here as they are still user-supplied. IMO it would be worthwhile to first collect the full names of all values in modules_to_save and then check if they are tied to save us from having various places where we do prefix/suffix/infix/whatever comparisons.

githubnemo · 2025-12-08T16:44:11Z

src/peft/tuners/lora/model.py

+
+        modules_to_save = getattr(peft_config, "modules_to_save", []) or []
+
+        embed_layer_name = find_parameter_name_by_tensor(self.model, self.model.get_input_embeddings().weight)


I think there is no guarantee that this will return the name of the embedding layer. It could also return the name of a layer tied to the embedding layer. It is probably safer to compare module identity instead (even though for transformers <5 this will also be flaky for models like T5).

githubnemo · 2025-12-08T16:47:42Z

src/peft/tuners/lora/model.py

+        # find_parameter_name_by_tensor returns the parameter name, so we need to strip the weight from the name
+        embed_layer_name = embed_layer_name.replace(".weight", "").replace("model.", "")


Not sure if replacing these strings is a good idea. encoder_model.embed_tokens would be turned into encoder_embed_tokens. Maybe using a more restricted approach (only one replacement, only if the key is found) would be better? .weight for example could be dropped by using .removesuffix.

githubnemo · 2025-12-08T16:55:40Z

src/peft/tuners/lora/model.py

+        """
+        Tied weight keys contains the layers tied to the embedding layer. Add embedding layer and remove rest of the
+        tied layers from `module_to_save`. Maintain a separate set for layers to be tied
+
+        Args:
+            peft_config (LoraConfig)
+            tied_weight_keys (list[str])
+        """


Let's remove or document the parameters from the docstring, simply listing them doesn't add value.

Something like this:

Suggested change

"""

Tied weight keys contains the layers tied to the embedding layer. Add embedding layer and remove rest of the

tied layers from `module_to_save`. Maintain a separate set for layers to be tied

Args:

peft_config (LoraConfig)

tied_weight_keys (list[str])

"""

"""

Add embedding layer to `modules_to_save` and remove rest of the tied layers from `module_to_save`.

Maintain a separate set for layers to be tied in `peft_config.tied_weights_keys`.

Args:

peft_config (LoraConfig)

tied_weight_keys (list[str])

Contains the layers tied to the embedding layer.

"""

But what I'm still missing is an explanation to some of this. Especially:

"Maintain a separate set for layers to be tied [in peft_config.tied_weights_keys.]"

Can you add to the docstring to what end this is done?

githubnemo · 2025-12-08T16:58:59Z

src/peft/tuners/lora/model.py

+            if m in modules_to_save:
+                modules_to_save.remove(m)


I'm not sure how often this will generate a match. If I understand correctly, tied_weight_keys are fully-qualified keys. So this check will only match if the keys in modules_to_save are also fully-qualified. I don't think this happens often. cc @BenjaminBossan

githubnemo · 2025-12-08T17:14:31Z

src/peft/tuners/lora/config.py

                "This will ensure that the adapters added to the tied layers "
                "are also tied. This is only applicable for layers passed via "
-                "`modules_to_save`."
+                "`modules_to_save` and and `target_modules`."


Suggested change

"`modules_to_save` and and `target_modules`."

"`modules_to_save` and `target_modules`."

githubnemo · 2025-12-08T17:17:27Z

src/peft/peft_model.py

+                # Before exporting the parameters we need to make sure
+                # all the tensors are contigious. Tensors can become non contigiuous


Suggested change

# Before exporting the parameters we need to make sure

# all the tensors are contigious. Tensors can become non contigiuous

# Before exporting the parameters we need to make sure all the tensors are contigious as saving

# non-contiguous parameters is not supported. Tensors can become non contigiuous

githubnemo · 2025-12-09T14:35:05Z

src/peft/tuners/lora/layer.py

        arrow_config: ArrowConfig = None,
        qalora_group_size: int = 32,
        inference_mode: bool = False,
+        tied_adapters: Optional[dict[str, nn.Parameter]] = None,


Is this a misnomer? IIUC it only contains LoRA parameters for one adapter?

githubnemo · 2025-12-09T15:10:25Z

src/peft/tuners/lora/model.py

-    def _add_modules_to_tie(self, peft_config, tied_weight_keys):
-        modules_to_save = set(getattr(peft_config, "modules_to_save", []) or [])
-        missing_keys = set(tied_weight_keys) - modules_to_save
+    def _add_modules_to_tie(self, peft_config: LoraConfig, tied_weight_keys: list[str]):


Now that we have _add_target_modules as well I'm wondering if we should refactor this to _add_modules_to_save_to_tie for clarity (it is verbose, yes).

Same goes for the config key modules_to_tie.

githubnemo · 2025-12-09T15:57:04Z

src/peft/tuners/lora/model.py

+        for m in tied_weight_keys:
+            if m in target_modules:
+                target_modules.remove(m)


This will also only occasionally match, right? Only if users supply the fully-qualified module names.

Tests and inital implementation for embed_tokens

4c6d15f

romitjain changed the title ~~Tie weights for target_modules in Lora~~ Tie weights for target_modules in Lora (#2864) Oct 29, 2025

BenjaminBossan reviewed Oct 29, 2025

View reviewed changes

romitjain added 2 commits October 30, 2025 09:52

Minor fixes

4b91220

Fixed all tests and made updates to logic

46b803e

Signed-off-by: romit <romit@ibm.com>

romitjain commented Oct 31, 2025

View reviewed changes

src/peft/tuners/tuners_utils.py Outdated Show resolved Hide resolved

romitjain commented Oct 31, 2025

View reviewed changes

src/peft/tuners/tuners_utils.py Show resolved Hide resolved

romitjain marked this pull request as ready for review October 31, 2025 12:11

romitjain changed the title ~~Tie weights for target_modules in Lora (#2864)~~ ENH: Tie weights for target_modules in Lora (#2864) Oct 31, 2025

romitjain requested a review from BenjaminBossan October 31, 2025 12:16

Nit

37b1e06

BenjaminBossan requested changes Nov 3, 2025

View reviewed changes

romitjain and others added 3 commits November 4, 2025 12:17

Added contigious check for export

8388aa8

Signed-off-by: romit <romit@ibm.com>

Apply suggestion from @BenjaminBossan

cd6c6d0

Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

Addressed PR comments

0cb44e8

Signed-off-by: romit <romit@ibm.com>

romitjain requested a review from BenjaminBossan November 5, 2025 12:20

BenjaminBossan requested changes Nov 6, 2025

View reviewed changes

romitjain and others added 6 commits November 7, 2025 10:31

Update src/peft/tuners/lora/model.py

628ce10

Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

Update src/peft/tuners/lora/model.py

602ce10

Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

Apply suggestions from code review

e2d0345

Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

Removed redundant change

7880032

Merge branch 'enh/tie-target-modules' of github.com:romitjain/peft in…

f73af50

…to enh/tie-target-modules

Handling target_modules as str

46cca1e

Signed-off-by: romit <romit@ibm.com>

romitjain commented Nov 7, 2025

View reviewed changes

tests/test_initialization.py Show resolved Hide resolved

romitjain requested a review from BenjaminBossan November 7, 2025 13:34

BenjaminBossan requested changes Nov 10, 2025

View reviewed changes

romitjain and others added 2 commits November 10, 2025 21:21

Update src/peft/tuners/tuners_utils.py

2267a48

Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

Updated regex matching

5d5b8e4

Signed-off-by: romit <romit@ibm.com>

romitjain requested a review from BenjaminBossan November 12, 2025 07:07

BenjaminBossan requested changes Nov 12, 2025

View reviewed changes

romitjain and others added 3 commits November 13, 2025 10:11

Apply suggestion from @BenjaminBossan

c7cfe40

Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>

Added find layer by tensor

8294ec7

Signed-off-by: romit <romit@ibm.com>

Merge branch 'main' of github.com:romitjain/peft into enh/tie-target-…

7370a21

…modules

romitjain requested a review from BenjaminBossan November 13, 2025 08:47

BenjaminBossan requested changes Nov 14, 2025

View reviewed changes

Fixed tests

1da895f

Signed-off-by: romit <romit@ibm.com>

romitjain requested a review from BenjaminBossan November 14, 2025 12:15

BenjaminBossan approved these changes Nov 18, 2025

View reviewed changes

tests/test_initialization.py Outdated Show resolved Hide resolved

Nit

d86ff7d

Signed-off-by: romit <romit@ibm.com>

Small fix to ensure correct layer name gets saved for target modules

dc03dd4

Signed-off-by: romit <romit@ibm.com>

Merge branch 'main' of github.com:huggingface/peft into enh/tie-targe…

c79a64c

…t-modules

githubnemo reviewed Dec 9, 2025

View reviewed changes


		modules_to_save = getattr(peft_config, "modules_to_save", []) or []

		embed_layer_name = find_parameter_name_by_tensor(self.model, self.model.get_input_embeddings().weight)

		# find_parameter_name_by_tensor returns the parameter name, so we need to strip the weight from the name
		embed_layer_name = embed_layer_name.replace(".weight", "").replace("model.", "")

	"`modules_to_save` and and `target_modules`."
	"`modules_to_save` and `target_modules`."

		# Before exporting the parameters we need to make sure
		# all the tensors are contigious. Tensors can become non contigiuous

ENH: Tie weights for target_modules in Lora (#2864) #2879

Are you sure you want to change the base?

ENH: Tie weights for target_modules in Lora (#2864) #2879

Uh oh!

Conversation

romitjain commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romitjain commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BenjaminBossan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romitjain commented Oct 31, 2025

Uh oh!

BenjaminBossan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romitjain commented Nov 5, 2025

Uh oh!

BenjaminBossan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

romitjain commented Nov 7, 2025

Uh oh!

BenjaminBossan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

romitjain commented Oct 29, 2025 •

edited

Loading

romitjain commented Oct 29, 2025 •

edited

Loading