-
Notifications
You must be signed in to change notification settings - Fork 6.5k
[modular] add tests for qwen modular #12585
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
base: main
Are you sure you want to change the base?
Conversation
| @property | ||
| def inputs(self) -> List[InputParam]: | ||
| return [ | ||
| InputParam("latents"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to how it's done in the other pipelines.
| ) | ||
| ] | ||
| * block_state.batch_size | ||
| for _ in range(block_state.batch_size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have two options:
- Either this
- Or how it's done in edit:
diffusers/src/diffusers/pipelines/qwenimage/pipeline_qwenimage_edit.py
Lines 752 to 757 in 325a950
img_shapes = [ [ (1, height // self.vae_scale_factor // 2, width // self.vae_scale_factor // 2), (1, calculated_height // self.vae_scale_factor // 2, calculated_width // self.vae_scale_factor // 2), ] ] * batch_size
Regardless, the current implementation isn't exactly the same as how the standard pipeline implements it and would break for the batched input tests we have.
| block_state = self.get_block_state(state) | ||
|
|
||
| # YiYi Notes: remove support for output_type = "latents', we can just skip decode/encode step in modular | ||
| vae_scale_factor = 2 ** len(components.vae.temperal_downsample) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping vae_scale_factor fixed to 8, for example, would break the tests as we use a smaller VAE.
| block_state.negative_prompt_embeds = None | ||
| block_state.negative_prompt_embeds_mask = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise, no CFG settings would break.
| @pytest.mark.xfail(condition=True, reason="Batch of multiple images needs to be revisited", strict=True) | ||
| def test_num_images_per_prompt(self): | ||
| super().test_num_images_per_prompt() | ||
|
|
||
| @pytest.mark.xfail(condition=True, reason="Batch of multiple images needs to be revisited", strict=True) | ||
| def test_inference_batch_consistent(): | ||
| super().test_inference_batch_consistent() | ||
|
|
||
| @pytest.mark.xfail(condition=True, reason="Batch of multiple images needs to be revisited", strict=True) | ||
| def test_inference_batch_single_identical(): | ||
| super().test_inference_batch_single_identical() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are skipped in the standard pipeline tests, too.
|
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. |
|
With Update: Checking with the All of those tests work with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just comments on aligning with new testing structure.
| from ..test_modular_pipelines_common import ModularPipelineTesterMixin | ||
|
|
||
|
|
||
| class QwenImageModularTests: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| class QwenImageModularTests: | |
| class QwenImageModularTesterMixin: |
| pipeline_class = QwenImageModularPipeline | ||
| pipeline_blocks_class = QwenImageAutoBlocks | ||
| repo = "hf-internal-testing/tiny-qwenimage-modular" | ||
|
|
||
| params = frozenset(["prompt", "height", "width", "negative_prompt", "attention_kwargs", "image", "mask_image"]) | ||
| batch_params = frozenset(["prompt", "negative_prompt", "image", "mask_image"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move these into the actual test object e.g QwenImageModularPipelineFastTests
| params = frozenset(["prompt", "height", "width", "negative_prompt", "attention_kwargs", "image", "mask_image"]) | ||
| batch_params = frozenset(["prompt", "negative_prompt", "image", "mask_image"]) | ||
|
|
||
| def get_pipeline(self, components_manager=None, torch_dtype=torch.float32): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can just use the one defined in ModularPipelineTesterMixin
| pipeline.set_progress_bar_config(disable=None) | ||
| return pipeline | ||
|
|
||
| def get_dummy_inputs(self, device, seed=0): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can move into actual test object e.g. QwenImageModularPipelineFastTests
| return inputs | ||
|
|
||
|
|
||
| class QwenImageModularGuiderTests: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| class QwenImageModularGuiderTests: | |
| class QwenImageModularGuiderTesterMixin: |
| class QwenImageModularPipelineFastTests( | ||
| QwenImageModularTests, QwenImageModularGuiderTests, ModularPipelineTesterMixin, unittest.TestCase |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unittest
| class QwenImageModularPipelineFastTests( | |
| QwenImageModularTests, QwenImageModularGuiderTests, ModularPipelineTesterMixin, unittest.TestCase | |
| class TestQwenImageModularPipelineFast( | |
| QwenImageModularTests, QwenImageModularGuiderTests, ModularPipelineTesterMixin |
|
I will wait for your #12579 PR to get merged first. Easier that way for me to apply your suggestions. |
What does this PR do?
Some things came up with the tests that needed fixing. But LMK if you would like me to revert them.
Will propagate the changes from #12579 once it's merged.