-
Notifications
You must be signed in to change notification settings - Fork 5.9k
Fixing missing provider options argument #11397
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?
Fixing missing provider options argument #11397
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.
Pull Request Overview
This PR fixes the missing provider options argument for diffusers pipelines, ensuring that the ONNX Runtime InferenceSession receives the expected list of provider options.
- Update the provider_options parameter to a list in the InferenceSession instantiation.
- Propagate provider_options from kwargs in both local and hub model loading.
return ort.InferenceSession( | ||
path, providers=[provider], sess_options=sess_options, provider_options=provider_options | ||
path, providers=[provider], sess_options=sess_options, provider_options=[provider_options] |
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.
Wrapping provider_options in a list may result in [None] if provider_options is not provided. Consider defaulting to an empty list or checking for a non-null value before wrapping.
Copilot uses AI. Check for mistakes.
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. |
@yiyixuxu Added changes for None case and explained second Copilot suggestions. |
@@ -75,6 +75,11 @@ def load_model(path: Union[str, Path], provider=None, sess_options=None, provide | |||
logger.info("No onnxruntime provider specified, using CPUExecutionProvider") | |||
provider = "CPUExecutionProvider" | |||
|
|||
if provider_options is None: | |||
provider_options = [] | |||
else: |
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.
do we need this?
else: | |
elif not isinstance(provider_options, list): | |
What does this PR do?
This PR adding support for provider options argument for diffusers pipelines. In previous approach, not all of the necessary code had been added following those two closed PRs:
#10719
#10661
Provider options argument is missing on two places:
diffusers/src/diffusers/pipelines/onnx_utils.py
lines 177. and 193.ONNXRuntime Inference Session( ) method accepts provider options as a list of dictionary, change on:
diffusers/src/diffusers/pipelines/onnx_utils.py
line 79.Fixes missing provider options argument.
Before submitting
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
@yiyixuxu
@asomoza
@sayakpaul
@DN6