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

Add support for diffuser pipelines in gr.Interface.from_pipeline(). #7265

Merged
merged 48 commits into from Mar 8, 2024

Conversation

shubhamofbce
Copy link
Contributor

@shubhamofbce shubhamofbce commented Feb 1, 2024

Description

I have added support for the following diffuser pipelines in gr.Interface.from_pipeline().

  • StableDiffusionPipeline
  • StableDiffusionImg2ImgPipeline
  • StableDiffusionInpaintPipeline
  • StableDiffusionDepth2ImgPipeline
  • StableDiffusionImageVariationPipeline
  • StableDiffusionInstructPix2PixPipeline
  • StableDiffusionUpscalePipeline

Closes: #6856

🎯 PRs Should Target Issues

Not adhering to this guideline will result in the PR being closed.

Tests

  1. PRs will only be merged if tests pass on CI. To run the tests locally, please set up your Gradio environment locally and run the tests: bash scripts/run_all_tests.sh

  2. You may need to run the linters: bash scripts/format_backend.sh and bash scripts/format_frontend.sh

@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Feb 1, 2024

🪼 branch checks and previews

Name Status URL
Spaces ready! Spaces preview
Website ready! Website preview
🦄 Changes detecting...

Install Gradio from this PR

pip install https://gradio-builds.s3.amazonaws.com/a64bc61e390b9c4a09a3300a4cd971b05587e2dc/gradio-4.20.1-py3-none-any.whl

Install Gradio Python Client from this PR

pip install "gradio-client @ git+https://github.com/gradio-app/gradio@a64bc61e390b9c4a09a3300a4cd971b05587e2dc#subdirectory=client/python"

@shubhamofbce shubhamofbce changed the title Add support for StableDiffusionPipeline Add support for Diffuser pipelines Feb 1, 2024
@shubhamofbce shubhamofbce changed the title Add support for Diffuser pipelines Add support for diffuser pipelines in gr.Interface.from_pipeline(). Feb 1, 2024
@shubhamofbce
Copy link
Contributor Author

@abidlabs, I've made an initial commit adding support for the Stable Diffusion pipeline, which I've successfully tested locally. Could you or someone from your team review the approach? I plan to apply the same methodology to the remaining pipelines.
Although the PR is still in draft, I'd like to ensure I'm on the right track early on, given the PR's substantial scope.

gradio/pipelines.py Outdated Show resolved Hide resolved
@abidlabs
Copy link
Member

abidlabs commented Feb 1, 2024

Thanks @shubhamofbce this is looking good! Just one comment at the top

@shubhamofbce shubhamofbce force-pushed the support-diffuser-pipleines branch 2 times, most recently from 992115f to 34209af Compare February 3, 2024 08:07
@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Feb 3, 2024

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
gradio minor
  • Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Add support for diffuser pipelines in gr.Interface.from_pipeline().

Maintainers or the PR author can modify the PR title to modify this entry.

Something isn't right?

  • Maintainers can change the version label to modify the version bump.
  • If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can update the changelog file directly.

@shubhamofbce shubhamofbce marked this pull request as ready for review February 3, 2024 08:11
@shubhamofbce
Copy link
Contributor Author

@aliabd This is ready for review.
I have made the changes such that if the pipeline you are trying to load is a transformer pipeline, only then you need to install transformers and same for diffusers. You don't need to install both of them together.
I can see test / python / linux failing, can you please confirm if that is expected or i need to do something as i have formatted my code using format_backend script, but it suggested a few reformatting in some part which were not my change so i didn't committed them. If it is failing due to that I will reformat the file and commit.

gradio/pipelines.py Outdated Show resolved Hide resolved
gradio/pipelines.py Outdated Show resolved Hide resolved
gradio/pipelines.py Outdated Show resolved Hide resolved
@abidlabs
Copy link
Member

abidlabs commented Feb 5, 2024

Thanks @shubhamofbce this generally looks good to me. I'll give it a full review after you've refactored based on @akx's comments.

To get the python tests to run properly, you'll need to use ruff to format the code. Please run bash scripts/format_backend.sh

@shubhamofbce shubhamofbce marked this pull request as draft February 9, 2024 15:51
@abidlabs
Copy link
Member

abidlabs commented Feb 9, 2024

@shubhamofbce let us know when this is ready for another look! Thanks

@shubhamofbce shubhamofbce marked this pull request as ready for review February 10, 2024 12:11
@shubhamofbce
Copy link
Contributor Author

@akx @abidlabs This is ready for review now.
I have created a new helper file to store the _helper methods and refactored the code to use new helper methods.

One important change that i want to point out
I have also changed the type of pipeline in the load_from_pipeline function from pipelines.base.Pipelines to Any.
I was left with only this choice as i tried using Union of pipelines.base.Pipelines and diffusers.pipelines.DiffusionPipeline but then Diffusion pipeline doesn't have a _call method due to which i was not able to use it as a type, as we are calling the class. So i was not able to come up with a common type for both these pipelines.
Let me know if i can use something better as the input type or if you have any better suggestions.

@shubhamofbce
Copy link
Contributor Author

I have also formatted the code using format_backend.sh but still the test / python / linux is failing not sure why.

@abidlabs
Copy link
Member

Thanks @shubhamofbce for making the changes! It looks like we need we need to add diffusers as a dependency to test/requirements.in and test/requirements.txt to make the typechecker aware of the diffusers library Would you be able to add to that? (That'll also allow us to write a test for this integration, which I can take care of)

@abidlabs abidlabs self-assigned this Feb 11, 2024
@shubhamofbce
Copy link
Contributor Author

@abidlabs I have updated the requirements.in file and also updated the requirements.txt file using pip compile.
Now there are some tests failing in routes.py and route_utils.py, None of them are related to my change, not sure how to handle them.
image

gradio/pipelines.py Outdated Show resolved Hide resolved
gradio/pipelines.py Outdated Show resolved Hide resolved
gradio/pipelines.py Outdated Show resolved Hide resolved
gradio/pipelines.py Outdated Show resolved Hide resolved
gradio/pipelines_helpers.py Outdated Show resolved Hide resolved
gradio/pipelines_helpers.py Outdated Show resolved Hide resolved
gradio/pipelines_helpers.py Outdated Show resolved Hide resolved
test/requirements.txt Outdated Show resolved Hide resolved
@shubhamofbce
Copy link
Contributor Author

shubhamofbce commented Feb 18, 2024

@akx I have fixed the code as per your suggestions, Also i have tested locally a transformer pipeline and a diffuser pipeline to make sure everything is working as expected.
I have used pip-compile --upgrade to generate requirements.txt as there were some conflicts due to which test / python were failing. Now it is working fine, there are some cases failing but they are not related to my changes.
Let me know if you have any other comments or suggestion on this PR.

@shubhamofbce
Copy link
Contributor Author

@akx and @abidlabs I have also added unit tests one for load transformers from pipeline and one to load diffusers from pipeline. This is to make sure there are no typos introduced while refactoring code. Please let me know if more such tests are required.

gradio/pipelines_helpers.py Outdated Show resolved Hide resolved
gradio/pipelines_helpers.py Outdated Show resolved Hide resolved
gradio/pipelines_helpers.py Outdated Show resolved Hide resolved
gradio/pipelines_helpers.py Outdated Show resolved Hide resolved
gradio/pipelines_helpers.py Outdated Show resolved Hide resolved
test/requirements.txt Show resolved Hide resolved
test/test_pipelines.py Outdated Show resolved Hide resolved
test/test_pipelines.py Outdated Show resolved Hide resolved
@shubhamofbce
Copy link
Contributor Author

@akx @abidlabs Just checking if we have any update on the reviews.

@abidlabs
Copy link
Member

abidlabs commented Mar 4, 2024

Thanks @shubhamofbce I'll review and get this merged in this week!

Copy link
Member

@abidlabs abidlabs left a comment

Choose a reason for hiding this comment

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

Great PR @shubhamofbce! Thank you so much for taking our feedback into account. I made some small tweaks, but overall, this looks good to me.

Will merge in once CI passes

@abidlabs abidlabs merged commit 6ebf0ce into gradio-app:main Mar 8, 2024
7 checks passed
@pngwn pngwn mentioned this pull request Mar 8, 2024
@abidlabs
Copy link
Member

@shubhamofbce your contribution is now part of the official diffusers docs: https://huggingface.co/docs/diffusers/main/en/api/pipelines/stable_diffusion/overview#create-web-demos-using-gradio

Thanks again for the great PR!

@shubhamofbce
Copy link
Contributor Author

Thanks @abidlabs, looking forward for more such useful additions to gradio.

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.

Add diffusers pipelines to gr.Interface.from_pipeline()
4 participants