-
Notifications
You must be signed in to change notification settings - Fork 28.6k
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 deepspeed with quantization #37324
Conversation
Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. The CI will be paused while the PR is in draft mode. When it is ready for review, please click the |
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. |
@Cyrilvallez, I validated that this PR still works for our use case. Let's wait for @winglian to confirm that this PR unbreaks their use-case. Also I'd insist on adding the missing test #37315 (comment) since clearly @winglian's use case isn't covered by deepspeed tests under |
All good on my end ✅ , thanks! we'll put together a follow up PR to add a test for the other deepspeed test cases. |
FYI: if necessary, feel free to use the comment
to trigger a CI running |
* Update modeling_utils.py * Update modeling_utils.py
* Update modeling_utils.py * Update modeling_utils.py
What does this PR do?
Alright, this should fix all deepspeed issues (finally). See here https://github.com/huggingface/transformers/pull/36963/files#diff-6b72b98c4c2dcfc6cc606843917733f5d858374fbc22a735ff483bbc0c1e63eaL4248-L4252,
low_cpu_mem_usage
was forbidden with deepspeed, but then still overriden here in the quantized case https://github.com/huggingface/transformers/pull/36963/files#diff-6b72b98c4c2dcfc6cc606843917733f5d858374fbc22a735ff483bbc0c1e63eaL4358-L4361cc @stas00, @winglian
@stas00, next steps will be making sure tests are robust, and once this is done we'll try to see how we can truly harmonize the paths, so that everything is taking the same one! But this will be done after the tests, to avoid potential similar issues as what happened these past days 🙃