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

Handle batches and Tensors in pipeline_stable_diffusion_inpaint.py:prepare_mask_and_masked_image #1003

Merged
merged 12 commits into from Nov 20, 2022

Conversation

vict0rsch
Copy link
Contributor

Addresses #1000

Not sure what your code conventions are, I've been quite verbose in comments

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Oct 26, 2022

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

@vict0rsch
Copy link
Contributor Author

Not sure why black isn't happy. Locally I see:

~/Documents/Github/vict0rsch/diffusers on  main ⌚ 15:42:20
$ black  --check --preview examples tests src utils scripts
All done! ✨ 🍰 ✨
174 files would be left unchanged.

@patrickvonplaten
Copy link
Contributor

In general this is ok for me! @patil-suraj could you also take a look?

@patrickvonplaten
Copy link
Contributor

Regarding the black failure, could you maybe update your black version:

pip install --upgrade black

and then run make black again?

Also could we add one test that shows how the code changes now work? You can add a test under tests/pipelines/stable_diffusion/pipeline_inpaint.py

neonfuzz added a commit to neonfuzz/image-ai-utils-server that referenced this pull request Nov 13, 2022
theoretically, this allows the inpainting pipeline to have batches, not
just single images

in practice, diffusers needs to allow for inpaint batching before this
will have any effect
huggingface/diffusers#1003
@patrickvonplaten
Copy link
Contributor

Hey @vict0rsch,

We can fix black if you want :-)

Could you maybe add a test that show-cases how this PR fixes something that currently doesn't work?

@patrickvonplaten
Copy link
Contributor

We already allow to pass images at pytorch tensors I think so I'm wondering whether we need to take care of this here

@patrickvonplaten
Copy link
Contributor

Linking this to #1128 as well ->

We need a test for these changes, as they touch highly used pipelines.
Could you add a code snippet show-casing which use case is currently not possible, but will be enabled by this PR?

@vict0rsch
Copy link
Contributor Author

Hey sorry I got busy and forgot because I have a cloned and patched local version. I'm going to work on this

upgrade `black`
@vict0rsch
Copy link
Contributor Author

@patrickvonplaten do you mean a new tests/pipelines/stable_diffusion/pipeline_inpaint.py or do you refer to the existing tests/pipelines/stable_diffusion/test_stable_diffusion_inpaint.py ?

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

@vict0rsch
Copy link
Contributor Author

vict0rsch commented Nov 16, 2022

I'm assuming you'll resolve conflicts? my bad it was because of my local black config

@vict0rsch
Copy link
Contributor Author

Actually I see tests are still not happy with black it's very weird because after upgrading to the latest black I continue to see

$ black  --check --preview examples tests src utils scripts
All done! ✨ 🍰 ✨
174 files would be left unchanged.

But the tests expect something different, on files I have not touched...

Run black  --check --preview examples tests src utils scripts
  black  --check --preview examples tests src utils scripts
  isort --check-only examples tests src utils scripts
  flake8 examples tests src utils scripts
  doc-builder style src/diffusers docs/source --max_len 119 --check_only --path_to_docs docs/source
  shell: /usr/bin/bash -e {0}
  env:
    pythonLocation: /opt/hostedtoolcache/Python/3.7.15/x64
    PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.7.15/x64/lib/pkgconfig
    Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.7.15/x64
    Python[2](https://github.com/huggingface/diffusers/actions/runs/3483685264/jobs/5827440704#step:5:2)_ROOT_DIR: /opt/hostedtoolcache/Python/[3](https://github.com/huggingface/diffusers/actions/runs/3483685264/jobs/5827440704#step:5:3).7.15/x6[4](https://github.com/huggingface/diffusers/actions/runs/3483685264/jobs/5827440704#step:5:4)
    Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.7.1[5](https://github.com/huggingface/diffusers/actions/runs/3483685264/jobs/5827440704#step:5:5)/x[6](https://github.com/huggingface/diffusers/actions/runs/3483685264/jobs/5827440704#step:5:6)4
    LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.[7](https://github.com/huggingface/diffusers/actions/runs/3483685264/jobs/5827440704#step:5:7).15/x64/lib
would reformat examples/community/imagic_stable_diffusion.py
would reformat examples/community/lpw_stable_diffusion.py
would reformat examples/community/lpw_stable_diffusion_onnx.py
would reformat examples/textual_inversion/textual_inversion.py
would reformat examples/textual_inversion/textual_inversion_flax.py

Oh no! 💥 💔 💥
5 files would be reformatted, 2[11](https://github.com/huggingface/diffusers/actions/runs/3483685264/jobs/5827440704#step:5:11) files would be left unchanged.
Error: Process completed with exit code 1.

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

@vict0rsch
Copy link
Contributor Author

Couple remarks:

  • may want to use it in pipeline_onnx_stable_diffusion_inpaint.py too
  • it's confusing that the input is (image, mask) but the output is (mask, masked_input)

@vict0rsch
Copy link
Contributor Author

13 tests pass with

pytest tests/pipelines/stable_diffusion/test_stable_diffusion_inpaint.py -k "StableDiffusionInpaintingPrepareMaskAndMaskedImageTests"

@patrickvonplaten
Copy link
Contributor

Thanks @vict0rsch - cool that you added all these tests

@patrickvonplaten patrickvonplaten merged commit 3bec90f into huggingface:main Nov 20, 2022
PhaneeshB pushed a commit to nod-ai/diffusers that referenced this pull request Mar 1, 2023
Co-authored-by: Elias Joseph <elias@nod-labs.com>
yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
…repare_mask_and_masked_image` (huggingface#1003)

* Handle batches and Tensors in `prepare_mask_and_masked_image`

* `blackfy`
upgrade `black`

* handle mask as `np.array`

* add docstring

* revert `black` changes with smaller line length

* missing ValueError in docstring

* raise `TypeError` for image as tensor but not mask

* typo in mask shape selection

* check for batch dim

* fix: wrong indentation

* add tests

Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
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.

None yet

4 participants