-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
GPTQ Integration #771
GPTQ Integration #771
Conversation
The documentation is not available anymore as the PR was closed or merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @SunMarc, Thank you for adding AutoGPTQ support 🚀. Left few comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot. This PR looks really good, I only have minor comments. I don't have any experience with GPTQ itself (yet), so I cannot really judge the more technical parts of the implementation.
A more general question: Does GPTQ generally not work with IA³ or is it just a matter of implementing it later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great, thanks a lot @SunMarc ! I left one comment
We can add docs and an example script later in a follow up PR
Thanks! 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR is looking already quite good from my POV. Unfortunately, the tests are still failing, I assume because they require the corresponding changes to land in transformers.
I have a few comments, but none of them are deal breakers.
LoraLayer.__init__( | ||
self, in_features=quant_linear_module.infeatures, out_features=quant_linear_module.outfeatures | ||
) | ||
self.quant_linear_module = quant_linear_module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an interesting deviation from how the other lora layers are implemented. Here, we pass the original layer (quant_linear_module
) and use it under the hood. For the normal Linear
lora layer, we don't get the layer, instead basically creating a new linear layer:
# in __init__
nn.Linear.__init__(self, in_features, out_features, **kwargs)
# in forward
result = F.linear(x, transpose(self.weight, self.fan_in_fan_out), bias=self.bias)
I actually prefer the solution here but wonder if there was a specific reason why this approach was not taken originally. If so, would that same reason apply here or are we good with having two different approaches? Hopefully, the others can clarify this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did that because I wanted to put the new QuantLinear class in the same place as the others for conformity. If we want to go with the same approach, we will need to create this new class in a function so that the auto_gptq
import is protected ( to avoid circular import as it is also importing peft ) . LMK what you think about this solution and I will add it in another PR.
I don't want to break the tests so i hardcoded the const value. I will change them back when we will have the next release of transformers |
What does this PR do ?
This PR adds the possibility to train lora + adalora adapters on top of GPTQ quantized model.
convert to peft model for training
save adapters after training
load saved adapters
to do