Skip to content

Conversation

@anton-l
Copy link
Member

@anton-l anton-l commented Sep 19, 2022

This will allow accelerate to reinitialize model outputs like so: output = UNet2DOutput({"sample": torch.randn(3, 4)}) and avoid nesting attributes.

Closes #560

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Sep 19, 2022

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

@patrickvonplaten
Copy link
Contributor

Now I see the need for this code 😅 . We need to adapt it though for diffusers.
I've deleted it here: https://github.com/huggingface/diffusers/pull/422/files (sorry should have explained better in the PR why)

But essentially, the code as is on this branch will currently break the following:

from diffusers import DDIMPipeline

pipeline = DDIMPipeline.from_pretrained("google/ddpm-cifar10-32")
outputs = pipeline()

outputs["images"]

@anton-l could you see how this can be fixed and then maybe also add a test here :-)

# check every way of getting the attribute
assert isinstance(outputs.images, np.ndarray)
assert outputs.images.shape == (1, 3, 4, 4)
assert isinstance(outputs["images"], np.ndarray)
Copy link
Member Author

@anton-l anton-l Sep 19, 2022

Choose a reason for hiding this comment

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

The transformers version (initial commit from this PR) was failing on this line
cc @patrickvonplaten

outputs = CustomOutput({"images": np.random.rand(1, 3, 4, 4)})

# check every way of getting the attribute
assert isinstance(outputs.images, np.ndarray)
Copy link
Member Author

@anton-l anton-l Sep 19, 2022

Choose a reason for hiding this comment

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

the pre-PR diffusers version was failing on this line

@anton-l
Copy link
Member Author

anton-l commented Sep 19, 2022

@patrickvonplaten I've implemented a simpler version that works for your case as well.
Also added high-coverage tests for every combination of inputs and outputs.



class ConfigTester(unittest.TestCase):
def test_outputs_single_attribute(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Very nice!

outputs = CustomOutput(images=[PIL.Image.new("RGB", (4, 4))])

# check every way of getting the attribute
assert isinstance(outputs.images, list)
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool that this is all tested now!

Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

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

Great! Thanks a lot for fixing it!

@anton-l anton-l merged commit a45dca0 into main Sep 20, 2022
@kashif kashif deleted the fix-nested-outputs branch September 22, 2022 15:06
yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
* Fix BaseOutput initialization from dict

* style

* Simplify post-init, add tests

* remove debug
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.

UNet2DOutput becomes nested when wrapped with accelerate

4 participants