Lazy import of bitsandbytes #1230
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, we imported from bitsandbytes eagerly if the package was installed. This caused two major issues: - Slow loading time of PEFT (~4 sec) - Errors with multiprocessing because bnb initializes CUDA This commit fixes both issues by importing bitsandbytes lazily. PEFT import time is now reduced to ~2sec. Notes Implementation-wise, I use a combination of local imports and module-level __getattr__. The latter was introduced in Python 3.7 and should therefore be safe to use. As for testing, it is not as straightforward as just adding a new unit test. The issue is that when multiple tests are run, depending on the order, bitsandbytes may already be imported once the new test runs. To avoid this issue, I created a completely separate testing script that is run in CI. If anyone has a better idea, please let me know.
Should not be necessary but better safe than sorry.
|
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. |
Normal CI doesn't support CUDA, d'uh.
pacman100
approved these changes
Dec 7, 2023
There was a problem hiding this comment.
Thank you @BenjaminBossan for fixes the issues as well as making the import faster, LGTM! 🚀
younesbelkada
approved these changes
Dec 7, 2023
.github/workflows/nightly.yml
Outdated
| @@ -34,7 +34,7 @@ jobs: | |||
| source activate peft | |||
| pip install -e . --no-deps | |||
| pip install pytest-reportlog | |||
|
|
|||
There was a problem hiding this comment.
Why this diff is here?
There was a problem hiding this comment.
I removed all trailing whitespace from the yml.
There was a problem hiding this comment.
Note that I moved the test to the proper unit tests using a trick that Zach showed, so I undid all the changes here (i.e. trailing whitespaces are back).
muellerzr
approved these changes
Dec 7, 2023
|
Thanks a lot @muellerzr for helping with the test. |
TaoSunVoyage
pushed a commit
to TaoSunVoyage/peft
that referenced
this pull request
Dec 14, 2023
Previously, we imported from bitsandbytes eagerly if the package was installed. This caused two major issues: - Slow loading time of PEFT (~4 sec) - Errors with multiprocessing because bnb initializes CUDA This commit fixes both issues by importing bitsandbytes lazily. PEFT import time is now reduced to ~2sec. Notes Implementation-wise, I use a combination of local imports and module-level __getattr__. The latter was introduced in Python 3.7 and should therefore be safe to use.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Should fix various issues reported here and elsewhere, e.g. #559, huggingface/accelerate#2216.
Description
Previously, we imported from bitsandbytes eagerly if the package was installed. This caused two major issues:
This commit fixes both issues by importing bitsandbytes lazily. PEFT import time is now reduced to ~2sec.
Notes
Implementation-wise, I use a combination of local imports and module-level
__getattr__. The latter was introduced in Python 3.7 and should therefore be safe to use.As for testing, it is not as straightforward as just adding a new unit test. The issue is that when multiple tests are run, depending on the order, bitsandbytes may already be imported once the new test runs. To avoid this issue, I created a completely separate testing script that is run in CI. It is executed on the nightly multi-GPU CI run. Locally, it passes, 🤞 that it also passes there.
If anyone has a better idea, please let me know.