Skip to content

Conversation

sayakpaul
Copy link
Member

What does this PR do?

#12283 should not have introduced a separate test for testing pipeline-level group offloading inference, as we already have one:

def test_group_offloading_inference(self):

This test modifies the existing test_group_offloading_inference() test to make use of the pipeline-level enable_group_offload().

@sayakpaul sayakpaul requested a review from DN6 September 15, 2025 08:27
# tiling is enabled and a forward pass is run, when accelerator streams are used, the execution order of
# the layers is not traced correctly. This causes errors. For apply group offloading to VAE, a
# warmup forward pass (even with dummy small inputs) is recommended.
exclude_modules = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we just set directly? Why nested loops? Or even set them via group_offloading_kwargs?

exclude_modules = ["vae", "vqvae", "image_encoder"]

Copy link
Member Author

Choose a reason for hiding this comment

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

Just readability convenience.

Passing exclude_modules = ["vae", "vqvae", "image_encoder"] is a bit misleading for the test, IMO. We would still log an info if non-member components are passed, though:

However, I have simplified this in a commit:

exclude_modules = {"vae", "vqvae", "image_encoder"}
exclude_modules = list(exclude_modules & set(pipe.components.keys()))

@sayakpaul sayakpaul requested a review from DN6 September 16, 2025 06:49
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.

2 participants