-
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
fix-gptq-training #1086
fix-gptq-training #1086
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.
Thank you very much for this @SunMarc !!
Small nit: python 3.7 is dead since june: https://devguide.python.org/versions/ so I would remove the checks you put in import_utils.py
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 for fixing this Marc. Apart from what Younes mentioned, I also have a few comments. Please check them out.
src/peft/import_utils.py
Outdated
return True | ||
else: | ||
raise ImportError( | ||
f"Found an incompatible version of auto-gptq. Found version {version_autogptq}, but only version above {AUTOGPTQ_MINIMUM_VERSION} are supported" |
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.
"but only version" > "but only versions"
Also, the line is very long, could you please add a line break?
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.
yes !
@@ -658,7 +658,7 @@ def setUp(self): | |||
from transformers import GPTQConfig | |||
|
|||
self.causal_lm_model_id = "marcsun13/opt-350m-gptq-4bit" | |||
self.quantization_config = GPTQConfig(bits=4, disable_exllama=True) | |||
self.quantization_config = GPTQConfig(bits=4, use_exllama=False) |
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.
Is there a chance that this will be supported in the future? If yes, we could add a TODO comment so that we don't forget about it.
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.
For exllama v1, i don't think it will be supported in the future as the author moved to exllamav2. I still have to try for exllamav2, so i can leave a todo comment for that.
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 for fixing these issues, LGTM.
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.
Awesome work on this quick fix @SunMarc !
What does this PR do ?
This PR updates the GPTQ training since the latest version of gptq includes breaking changes. We also switch
disable_exllama
touse_exllama
since it is deprecated in the latest version of transformers. Tests are passing with transformers main.