KEMBAR78
[tests] add test for hotswapping + compilation on resolution changes by sayakpaul · Pull Request #11825 · huggingface/diffusers · GitHub
Skip to content

Conversation

sayakpaul
Copy link
Member

What does this PR do?

As promised :)

@sayakpaul sayakpaul requested a review from BenjaminBossan June 28, 2025 07:30
@sayakpaul
Copy link
Member Author

Cc: @StrongerXi and @anijain2305 as well.

@sayakpaul sayakpaul requested a review from yiyixuxu June 28, 2025 07:31
@sayakpaul sayakpaul added performance Anything related to performance improvements, profiling and benchmarking torch.compile labels Jun 28, 2025
new_model = self.model_class.from_pretrained(tmp_dir, device_map="auto", max_memory=max_memory)
# Making sure part of the model will actually end up offloaded
self.assertSetEqual(set(new_model.hf_device_map.values()), {0, 1})
print(f" new_model.hf_device_map:{new_model.hf_device_map}")
Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated but hopefully okay :-)

Comment on lines 2121 to 2127
if different_resolutions is not None:
for height, width in self.different_shapes_for_compilation:
new_inputs_dict = self.prepare_dummy_input(height=height, width=width)
_ = model(**new_inputs_dict)
else:
output0_after = model(**inputs_dict)["sample"]
assert torch.allclose(output0_before, output0_after, atol=tol, rtol=tol)
Copy link
Member Author

Choose a reason for hiding this comment

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

We don't need assertions when testing for different shapes as that would make the test unnecessarily complicated. We just check for recompilations.

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Member

@BenjaminBossan BenjaminBossan left a comment

Choose a reason for hiding this comment

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

Thanks for extending the hotswapping tests. Overall looks good, but I have some suggestions.

@sayakpaul sayakpaul requested a review from BenjaminBossan June 30, 2025 13:49
Copy link
Member

@BenjaminBossan BenjaminBossan left a comment

Choose a reason for hiding this comment

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

Thanks, I learned something new about torch.compile. LGTM.

> [!TIP]
> Move your code inside the `with torch._dynamo.config.patch(error_on_recompile=True)` context manager to detect if a model was recompiled. If a model is recompiled despite following all the steps above, please open an [issue](https://github.com/huggingface/diffusers/issues) with a reproducible example.
If you expect to varied resolutions during inference with this feature, then make sure set `dynamic=True` during compilation. Refer to [this document](../optimization/fp16#dynamic-shape-compilation) for more details.

Choose a reason for hiding this comment

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

I wonder if we should publicize use_duck_shape = False as well...

Copy link
Collaborator

@yiyixuxu yiyixuxu left a comment

Choose a reason for hiding this comment

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

thanks

@sayakpaul sayakpaul merged commit 87f83d3 into main Jul 1, 2025
29 checks passed
@sayakpaul sayakpaul deleted the resolution-hotswap-tests branch July 1, 2025 04:14
tolgacangoz pushed a commit to tolgacangoz/diffusers that referenced this pull request Jul 5, 2025
…uggingface#11825)

* add resolution changes tests to hotswapping test suite.

* fixes

* docs

* explain duck shapes

* fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Anything related to performance improvements, profiling and benchmarking torch.compile

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants