Skip to content

FIX CI: Targeting fused parameters with LoRA - #5430

Merged
albertvillanova merged 1 commit into
huggingface:mainfrom
BenjaminBossan:fix-lora-targeting-fused-parameter
Apr 1, 2026
Merged

albertvillanova merged 1 commit into
huggingface:mainfrom
BenjaminBossan:fix-lora-targeting-fused-parameter

Conversation

@BenjaminBossan

@BenjaminBossan BenjaminBossan commented Apr 1, 2026 •

Copy link
Copy Markdown
Member

What does this PR do?

In this test, LoRA was targeting up_proj but not gate_proj. In Transformers, these two layers are now fused. Therefore, it is no longer possible to target just one of the two. The test is now changed to target both.

It would be possible to add a workaround and set zeros for the layer that is not being targeted, but that would be a bigger task. So if we can accept targeting both, I propose to just do that.

Note that during creation of the error message alerting the user to the fact that targeting just one is not possible, there was an error in PEFT. This error is being resolved via huggingface/peft#3127. That PR also fixes the KeyError: 'qwen2_vl' we see in the dev dependency CI.

Fixes subsequent issue found in #5429 (comment):

ValueError: Cannot convert PEFT target(s) up_proj without also targeting gate_proj because they are fused into gate_up_proj.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a GitHub issue? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

AI writing disclosure

We welcome the use of AI tools to help with contributions. For transparency and to help us improve our review process, please indicate the level of AI involvement in this PR.

  • No AI usage: the PR was written entirely by a human.
  • AI-assisted: some parts were suggested or improved by AI, but the PR was written and reviewed by a human.
  • AI-generated: the PR was mostly or fully generated by an AI tool.

Who can review?

@albertvillanova


Note

Low Risk
Low risk: test-only change that broadens LoRA target modules to align with upstream fused layers, with no runtime/library behavior changes.

Overview
Adjusts the RewardTrainer MoE PEFT regression test to include gate_proj alongside up_proj in LoraConfig(target_modules=...), reflecting that these projections are now fused in recent Transformers and cannot be targeted independently.

Written by Cursor Bugbot for commit 2265519. This will update automatically on new commits. Configure here.

See huggingface#5429

In this test, LoRA was targeting up_proj but not gate_proj. In
Transformers, these two layers are now fused. Therefore, it is no longer
possible to target just one of the two. The test is now changed to
target both.

It would be possible to add a workaround and set zeros for the layer
that is not being targeted, but that would be a bigger task. So if we
can accept targeting both, I propose to just do that.
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

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

Copy link
Copy Markdown
Member Author

@kashif It looks like the failing smoke test is unrelated and the dev tests are awaiting the PEFT fix linked above. If that's okay, feel free to merge.

@albertvillanova albertvillanova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks. I'm merging this.

@albertvillanova albertvillanova changed the title FIX Targeting fused parameters with LoRA FIX CI: Targeting fused parameters with LoRA Apr 1, 2026
@albertvillanova
albertvillanova merged commit 1ad25f9 into huggingface:main Apr 1, 2026
17 of 19 checks passed
@BenjaminBossan
BenjaminBossan deleted the fix-lora-targeting-fused-parameter branch April 2, 2026 09:06
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.

4 participants