-
Notifications
You must be signed in to change notification settings - Fork 177
fix: implement lazy imports for BetterTransformer #619
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
base: main
Are you sure you want to change the base?
fix: implement lazy imports for BetterTransformer #619
Conversation
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.
PR Summary
Implements lazy loading pattern for BetterTransformer in libs/infinity_emb/infinity_emb/transformer/acceleration.py to resolve import conflicts with newer transformers versions (>=4.49).
- Introduced
_import_bettertransformer()function for on-demand imports of BetterTransformer components - Added version compatibility checks for transformers library
- Wrapped BetterTransformer imports in try/except blocks for graceful failure handling
- Improved early return logic when BetterTransformer is disabled via
--no-bettertransformerflag
1 file reviewed, 1 comment
Edit PR Review Bot Settings | Greptile
| ) | ||
| try: | ||
| model = BetterTransformer.transform(model) | ||
| model = BetterTransformer.transform(model) # type: ignore |
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.
style: The type: ignore comment should explain why the type is being ignored, e.g. # type: ignore[attr-defined] since BetterTransformer could be None
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #619 +/- ##
=======================================
Coverage 79.54% 79.54%
=======================================
Files 43 43
Lines 3486 3501 +15
=======================================
+ Hits 2773 2785 +12
- Misses 713 716 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
michaelfeil
left a comment
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.
@wirthual can you check this out? Maybe we should vendor / maintain bettertransformer. Bettertransformer is ~50% faster than regular torch.
|
Absolutely. According to this issue:
What are your thoughts on this method vs continue support for current bettertransformer? |
|
@michaelfeil One option to keep bettertransformer as was is #641. Extracted bettertransformer code in its own package. This should get rid of version error while keeping the orig bettertransformer code. Not sure if/how long this method can work. |
Related Issue
Fixes import errors when using
--no-bettertransformerflag with transformers >= 4.49.Summary
This PR implements lazy imports for BetterTransformer to prevent import errors when it's disabled or when using incompatible transformers versions. BetterTransformer is deprecated in optimum and requires transformers < 4.49.
Changes:
_import_bettertransformer()function for on-demand imports--no-bettertransformeris used, avoiding unnecessary importsWhy this is needed:
--no-bettertransformerChecklist
Additional Notes
This change ensures backward compatibility for users who still want BetterTransformer while fixing the issue for users with newer dependencies.