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

fix lora modules_to_save issue #343

Merged
merged 2 commits into from
Apr 20, 2023
Merged

Conversation

pacman100
Copy link
Contributor

What does this PR do?

  1. Currently, modules_to_save aren't working when using LoRA. This PR fixes it. modules_to_save: "ValueError: Attempting to unscale FP16 gradients" #341 (comment)

@pacman100 pacman100 requested a review from younesbelkada April 20, 2023 09:36
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Apr 20, 2023

The documentation is not available anymore as the PR was closed or merged.

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 fixing! 🔥
Left one question, otherwise LGTM

@@ -68,7 +68,7 @@
class ClassInstantier(OrderedDict):
def __getitem__(self, key, *args, **kwargs):
# check if any of the kwargs is inside the config class kwargs
if any([kwarg in self[key][1] for kwarg in kwargs]):
if any(kwarg in self[key][1] for kwarg in kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why this is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ruff raises quality issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok I see, I think it's ok to merge it as it is,

@pacman100 pacman100 merged commit 9413b55 into main Apr 20, 2023
@pacman100 pacman100 deleted the smangrul/fix-lora-modules-to-save branch May 3, 2023 19:38
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