8000 added support for Conv1d for DoRA by EskildAndersen · Pull Request #2531 · huggingface/peft · GitHub
[go: up one dir, main page]
Skip to content

added support for Conv1d for DoRA #2531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
May 12, 2025
Merged

added support for Conv1d for DoRA #2531

merged 6 commits into from
May 12, 2025

Conversation

EskildAndersen
Copy link
Contributor

No description provided.

BenjaminBossan
BenjaminBossan previously requested changes May 5, 2025
View reviewed changes
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 working on the support of nn.Conv1d* for DoRA. Before proceeding, let's add unit tests for this feature. This is as simple as adding a line like this:

("Conv1d LoRA Conv1d with DoRA", "Conv1d", LoraConfig, {"target_modules": ["conv1d"], "use_dora": True}),

here:

peft/tests/test_custom_models.py

Line 115 in 6c054d0

("Conv1d LoRA", "Conv1d", LoraConfig, {"target_modules": ["conv1d"]}),

*Just to be clear, we already support transformers.pytorch_utils.Conv1D, which is a different layer type.

@HuggingFaceDocBuilderDev
Copy link

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.

@githubnemo
Copy link
Collaborator

Thanks, could you please run make style from the repository root directory and fix any issue that gets reported so that the CI 8000 passes?

@githubnemo
Copy link
Collaborator

@EskildAndersen please make sure to use the same ruff version as specified in setup.py or otherwise make style will potentially differ from the repository style.

either run

# in peft directory
pip install .[quality]

or

pip install ruff~=0.9.2

githubnemo
githubnemo approved these changes May 12, 2025
View reviewed changes
Copy link
Collaborator
@githubnemo githubnemo left a comment

Choose a reason for hiding this comment

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

LGTM. I suggest to not delete the clarifying comment about the handling of Conv layers but otherwise I think that this can be merged. I'll add the suggestion and once the pipeline is green, I'll merge it.

Thanks for the contribution, good work :)

@githubnemo githubnemo dismissed BenjaminBossan’s stale review May 12, 2025 18:02

I took over this PR and the comments were adressed.

@githubnemo githubnemo merged commit 8af29c6 into huggingface:main May 12, 2025
14 checks passed
@githubnemo githubnemo mentioned this pull request May 21, 2025
Closed
efraimdahl pushed a commit to efraimdahl/peft that referenced this pull request Jul 12, 2025
@EskildAndersen @efraimdahl
added support for Conv1d for DoRA (huggingface#2531)
20953cd 
DoRA now supports Conv1d layers and, notably, the check for how to deal with other than linear layers was softened from checking for 4 dimensions to now 3 dimensions since `Conv1d` layers have 3 elements instead of 4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@githubnemo githubnemo githubnemo approved these changes

@BenjaminBossan BenjaminBossan BenjaminBossan left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@EskildAndersen @HuggingFaceDocBuilderDev @githubnemo @BenjaminBossan
0