Skip to content

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

Merged
patrickvonplaten merged 12 commits into
huggingface:mainfrom
vict0rsch:main
Nov 20, 2022
Merged

Handle batches and Tensors in pipeline_stable_diffusion_inpaint.py:prepare_mask_and_masked_image#1003
patrickvonplaten merged 12 commits into
huggingface:mainfrom
vict0rsch:main

Conversation

@vict0rsch
Copy link
Copy Markdown
Contributor

@vict0rsch vict0rsch commented Oct 26, 2022

Addresses #1000

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

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

HuggingFaceDocBuilderDev commented Oct 26, 2022 •
edited
Loading

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

@vict0rsch
Copy link
Copy Markdown
Contributor Author

vict0rsch commented Oct 26, 2022

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
Copy Markdown
Contributor

patrickvonplaten commented Oct 29, 2022

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

@patrickvonplaten
Copy link
Copy Markdown
Contributor

patrickvonplaten commented Oct 29, 2022

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
@neonfuzz
Minor bugfix
a817755 
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
@neonfuzz neonfuzz mentioned this pull request Nov 13, 2022
Open
@vvvm23 vvvm23 mentioned this pull request Nov 14, 2022
Closed
@patrickvonplaten
Copy link
Copy Markdown
Contributor

patrickvonplaten commented Nov 16, 2022

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
Copy Markdown
Contributor

patrickvonplaten commented Nov 16, 2022

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
Copy Markdown
Contributor

patrickvonplaten commented Nov 16, 2022

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
Copy Markdown
Contributor Author

vict0rsch commented Nov 16, 2022

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

@vict0rsch
blackfy
c6f6dc3 
upgrade `black`
@vict0rsch
Copy link
Copy Markdown
Contributor Author

vict0rsch commented Nov 16, 2022

@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
Copy link
Copy Markdown

HuggingFaceDocBuilderDev commented Nov 16, 2022

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

@vict0rsch
Copy link
Copy Markdown
Contributor Author

vict0rsch commented Nov 16, 2022 •
edited
Loading

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

@vict0rsch
Copy link
Copy Markdown
Contributor Author

vict0rsch commented Nov 16, 2022

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
Copy link
Copy Markdown

HuggingFaceDocBuilderDev commented Nov 16, 2022

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

@vict0rsch
Copy link
Copy Markdown
Contributor Author

vict0rsch commented Nov 16, 2022

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
Copy Markdown
Contributor Author

vict0rsch commented Nov 16, 2022

13 tests pass with

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

@patrickvonplaten
Copy link
Copy Markdown
Contributor

patrickvonplaten commented Nov 20, 2022

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
@Eliasj42
added an example to run sharded bloom (huggingface#1003)
87fd13d 
Co-authored-by: Elias Joseph <elias@nod-labs.com>
yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
@vict0rsch @patrickvonplaten
Handle batches and Tensors in `pipeline_stable_diffusion_inpaint.py:p…
e17e11e 
…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

Reviewers

@patil-suraj patil-suraj Awaiting requested review from patil-suraj

Assignees

@patil-suraj patil-suraj

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vict0rsch @HuggingFaceDocBuilderDev @patrickvonplaten @patil-suraj