Skip to content
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.

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

TST Add regression test for DoRA, VeRA, BOFT, LN Tuning #1792

Conversation

BenjaminBossan
Copy link
Member

These new methods were added but the regression tests were not extended yet. This PR adds regression tests for these methods. The regression artifacts have been pushed based on PEFT v0.11.1. The new tests pass locally.

These new methods were added but the regression tests were not extended
yet. This PR adds regression tests for these methods. The regression
artifacts have been pushed based on PEFT v0.11.1. The new tests pass
locally.
@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
Contributor

@younesbelkada younesbelkada left a comment

Choose a reason for hiding this comment

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

Thanks a lot for adding these regression tests !

@BenjaminBossan BenjaminBossan merged commit 39c60ff into huggingface:main May 27, 2024
14 checks passed
@BenjaminBossan BenjaminBossan deleted the tst-regression-tests-dora-vera-boft-ln_tuning branch May 27, 2024 10:00
BenjaminBossan added a commit that referenced this pull request May 31, 2024
This PR moves all the DoRA functionality into a separate module class.
Essentially, this is necessary because otherwise, the DoRA parameter
lives on the lora.Linear layer as a parameter, not a separate module.
Since FSDP auto wrap policy operates on the level of modules, not
parameters, there is no way to modify the auto wrap policy to wrap the
DoRA parameter, it must be its own module.

If not for this reason, #1797 would be preferable, since the number of
code changes is smaller overall. In this PR, there are more numerous
changes, but the majority only involves moving code around, not any
actual code changes.

Since we introduce a new submodule, an extra steps are required to
ensure that old DoRA state dicts can still be loaded correctly. This
involves a fairly trivial extra remapping step in
set_peft_model_state_dict. The test for this is performed via the new
regression DoRA tests introduced in #1792.

Similarly, there is a remapping step involved in
get_peft_model_state_dict to ensure that when new state dicts with DoRA
are saved, they still conform to the old format.

An additional required change was to make a defensive copy of the base
layer before dequantizing its weight in order to calculate the weight
norm for DoRA. Without this defensive copy, some side-effect is
triggered in FSDP that results in

> ValueError: Cannot flatten integer dtype tensors

even though the compute dtype of bnb is correctly set to float.

Creating a fully functioning deepcopy does currently not work with 8bit
BNB but there is a fix. Once the next BNB release is out, 8bit BNB will
be tested and enabled.

While working on this, I also noticed a small bug that dropout was not
correctly applied when using QDoRA. This is now also fixed.

This PR was tested successfully with FSDP and (Q)DoRA using the scripts
in examples/sft/ with a modification to enable DoRA.
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