Skip to content

Pull Request: Adding HiRA integration into PEFT library#2668

Open
hqsiswiliam wants to merge 105 commits intohuggingface:mainfrom
hqsiswiliam:main
Open

Pull Request: Adding HiRA integration into PEFT library#2668
hqsiswiliam wants to merge 105 commits intohuggingface:mainfrom
hqsiswiliam:main

Conversation

@hqsiswiliam
Copy link

Feature request

This request proposes integrating HiRA (Hadamard High-Rank Adaptation) as described in the ICLR 2025 oral paper (https://openreview.net/pdf?id=TwJrTz9cRS) (https://iclr.cc/virtual/2025/oral/31839) and implemented in the hqsiswiliam/hira repository into the core PEFT library. This will enable users to apply HiRA through the familiar get_peft_model API and benefit from its high-rank updates without adding any inference overhead.

Motivation

General Motivation

PEFT methods like LoRA achieve parameter-efficient fine-tuning by injecting low-rank updates into pre-trained weights. While effective, purely low-rank adaptation can struggle to capture complex patterns in large language models.

1. Expressiveness grows with the rank

Empirically, increasing the LoRA rank in LLM training yields better downstream performance:

LoRA performance vs. rank
Higher LoRA rank correlates with improved task accuracy.

2. HiRA: Hadamard high-rank updates without extra parameters

HiRA sidesteps the expressiveness constraint by computing a Hadamard-enhanced update:

$$ \Delta W = W_0 \odot (A B) $$

HiRA update formula
HiRA uses the Hadamard product to inject high-rank structure into the frozen weight matrix $W_0$ via low-rank matrix $A$ and $B$.

3. Singular-value patterns

After training, HiRA exhibits a rich singular-value pattern, akin to full-rank fine-tuning (FFT), indicating its ability to model complex transformations without the expensive computational overhead:

Singular-value pattern comparison
HiRA’s singular-value distribution closely mirrors that of FFT.

4. Performance gains

Across commonsense reasoning benchmarks, HiRA outperforms LoRA and other PEFT baselines:

Commonsense reasoning performance
HiRA delivers notable accuracy improvements over baseline adapters.

5. No extra parameter or compute cost

Despite its high-rank behaviour, HiRA introduces no additional trainable parameters compared to LoRA:

Resource comparison: LoRA vs. HiRA
HiRA matches LoRA’s GRAM usage and training hours.

6. Complementary with LoRA (HiLoRA)

Combining HiRA and LoRA into a hybrid “HiLoRA” setup yields even stronger results than either method alone:

HiLoRA concept
HiLoRA performance gains
HiLoRA leverages both low-rank and Hadamard high-rank updates for better expressiveness.


By integrating HiRA into PEFT, users gain richer adaptation capability without sacrificing the parameter efficiency and usability that PEFT provides.

Your contribution

We would be pleased to submit a pull request to integrate HiRA class implementation into the PEFT framework. We welcome any suggestions for alternative integration approaches and appreciate any guidance on best practices.

Copy link
Member

@BenjaminBossan BenjaminBossan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR to add HiRA to PEFT. The method looks promising and the provided code is already quite mature.

When I started reading the paper, I was at first reminded of FedPara, aka LoHa, which is already integrated into PEFT, as that method also relies on the Hadamard product. However, IIUC, the two methods are still distinct: HiRA basically corresponds to LoRA, but instead of adding dW, we multiply it. In that way, it is much closer to LoRA than to LoHa. Still, I wanted to flag this, as I'm not sure you are aware (your paper doesn't seem to be reference FedPara).

At the moment, I haven't done a full in-depth review, but I think that makes more sense once we have completed the next step.

I noticed that you have formatted some unrelated files in method_comparison, could you please undo those changes? Usually, when you run make style, that directory should not be included.

I think a good next step is to add HiRA to the testing matrix we have in PEFT. For now, let's add some entries similar to the ones you can find here:

("Vanilla MLP 1 LoRA", "MLP", LoraConfig, {"target_modules": "lin0"}),
("Vanilla MLP 2 LoRA", "MLP", LoraConfig, {"target_modules": ["lin0"]}),
("Vanilla MLP 3 LoRA", "MLP", LoraConfig, {"target_modules": ["lin1"]}),

Since you also support embedding and conv layers, please make sure to include examples with those layers as well (basically, copy the relevant examples from LoRA and adjust them).

Then, please run pytest tests/test_custom_models.py -k "hira and not shira" -v and see if those tests pass. Once we get there, we can discuss the best next steps.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

@BenjaminBossan
Copy link
Member

@hqsiswiliam Do you still plan on working on this PR?

@hqsiswiliam
Copy link
Author

@hqsiswiliam Do you still plan on working on this PR?

Hi, BenjaminBossan. Thanks for checking in! I’ll continue working on this PR over the next few days.

- modify config.__class__ to config_cls
- remove _check_merge_allowed in hira model.
- adding HiRA cases to `MULTIPLE_ACTIVE_ADAPTERS_TEST_CASES`.
@BenjaminBossan
Copy link
Member

@hqsiswiliam There is a merge conflict, could you please take care? Also, once you're finished with your changes, please ping me for another review.

# Resolve Conflicts:
-	tests/test_custom_models.py
@hqsiswiliam
Copy link
Author

@BenjaminBossan Thanks for your update and the earlier review. I’ve resolved the merge conflict, and the changes are ready for another review. 😃

Copy link
Member

@BenjaminBossan BenjaminBossan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a small error in the docs. Also, could you please run make style to ensure that the linter is happy?

@hqsiswiliam
Copy link
Author

There is a small error in the docs. Also, could you please run make style to ensure that the linter is happy?

Hi @BenjaminBossan, thank you for the helpful comments and review. I’ve addressed the documentation issue and run make style, and the linter is now passing. I’ve also merged the latest main branch. Please let me know if there’s anything else you think would be helpful to adjust.

Copy link
Member

@BenjaminBossan BenjaminBossan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates. There are still a few tests that are failing, could you please check?

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

@hqsiswiliam
Copy link
Author

Hi, shall we continue to do the integration? 😊

@BenjaminBossan
Copy link
Member

Hi @hqsiswiliam could you please merge with/rebase on latest main?

@hqsiswiliam
Copy link
Author

Hi @hqsiswiliam could you please merge with/rebase on latest main?

Certainly, I will merge with the latest main in a few days.

@hqsiswiliam
Copy link
Author

Hi @BenjaminBossan, I’ve merged the latest main into this branch. Please let me know if any additional changes are needed.

@HuggingFaceDocBuilderDev

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.

Copy link
Member

@BenjaminBossan BenjaminBossan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the PR. Generally, not much is missing, but I found a few smaller issues, please check. A few HiRA tests are still failing, could you please check and resolve these errors?

On top of this, let's also add tests to test_config.py, test_decoder_models.py, test_feature_extraction_models.py, and test_seq_classifier.py. Moreover, I would strongly suggest to add an experiment to the MetaMathQA benchmark. This has the advantage that we can run an experiment to check that training works as expected and is more or less aligned with the expectations from the paper.

@hqsiswiliam
Copy link
Author

Thank you for your review. I have updated the code accordingly. I will also look into integrating an experiment on the MetaMathQA benchmark following the provided guidelines.

Copy link
Member

@BenjaminBossan BenjaminBossan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the latest updates.

Thank you for your review. I have updated the code accordingly. I will also look into integrating an experiment on the MetaMathQA benchmark following the provided guidelines.

Sounds good. Please ping me once that is done, + the missing unit tests I mentioned in my last comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what happened here, but GH shows the whole file being changed. Could you please check out the original file and re-apply your changed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved: LF was converted to CRLF; now restored.

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.

3 participants