Skip to content
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

Set default value to unet config sample size #1223

Merged
merged 1 commit into from
Jul 24, 2023
Merged

Conversation

echarlaix
Copy link
Collaborator

@echarlaix echarlaix commented Jul 24, 2023

Set default value for model without any config such as tiny-random-OnnxStableDiffusionPipeline. If no configuration provided, the default height and width value will be 512

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jul 24, 2023

The documentation is not available anymore as the PR was closed or merged.

@echarlaix echarlaix requested a review from fxmarty July 24, 2023 12:47
@fxmarty
Copy link
Contributor

fxmarty commented Jul 24, 2023

This seem different compared to diffusers that uses height = height or self.unet.config.sample_size * self.vae_scale_factor? Should we just add the relevant config to the tiny model?

@echarlaix
Copy link
Collaborator Author

echarlaix commented Jul 24, 2023

This seem different compared to diffusers that uses height = height or self.unet.config.sample_size * self.vae_scale_factor? Should we just add the relevant config to the tiny model?

This will break compatibility with models that have been exported to ONNX using diffusers or optimum<=v1.9.1 (we will no longer be able to perform inference with these models)

@fxmarty
Copy link
Contributor

fxmarty commented Jul 24, 2023

Shouldn't this fix be in diffusers?

@echarlaix
Copy link
Collaborator Author

Shouldn't this fix be in diffusers?

I'm talking about models that have already been exported such as https://huggingface.co/axodoxian/stable_diffusion_onnx or https://huggingface.co/sayakpaul/onnx-stable-diffusion-v1-5

Copy link
Contributor

@fxmarty fxmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@echarlaix
Copy link
Collaborator Author

Merging as failing tests are unrelated

@echarlaix echarlaix merged commit 29675d5 into main Jul 24, 2023
59 of 66 checks passed
@echarlaix echarlaix deleted the config-default branch July 24, 2023 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants