-
Notifications
You must be signed in to change notification settings - Fork 487
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
better handle noninstalled libs error during model loading #2071
base: main
Are you sure you want to change the base?
Conversation
can you give an example of the confusing error message you get with the current method ? |
File "/home/ea/miniforge3/lib/python3.12/site-packages/optimum/exporters/tasks.py", line 2066, in get_model_from_task |
Yes I agree for the |
I catch only ModuleNotFound exception for this case, so failure due to any other reason (e.g. issues with tokenizers) still will fail with original error, I do not see any problems and misleading here But if you insist, I can stay only for diffusers case |
I installed tokenizers==0.19 and ran the following:
It gives better way to handle the issue than simply |
@IlyasMoutawwakil ImportError != ModuleNotFoundError, so you still get your desired error message, ther is no any problem for your provided case |
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.
LGTM, thnaks !
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. |
@eaidova can you run styling please |
I tried to fix styling from here but didn't work.. 😅 |
@@ -2063,10 +2070,13 @@ def get_model_from_task( | |||
model_class_name = config.architectures[0] | |||
|
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.
Could make sense to have function that works for all supported libraries (diffusers / timm / sentence-transformers), taking instead the library_name
and doing a check with is_xxxxx_available
, so that we can call this function and do this check for all libraries anytime needed
What does this PR do?
I found an issue with loading models from extra-packages libraries (e.g. diffusers). TaskManager select and trying to load model even library is not installed in user environment and error message can be confusing for that case. This PR provides small improvement for this case.
Before submitting
Who can review?
@echarlaix, @IlyasMoutawwakil