[tests] refactor flux control lora tests - #14557
Conversation
dg845
left a comment
dg845
left a comment
There was a problem hiding this comment.
Thanks! Left some small comments.
| ) | ||
|
|
||
|
|
||
| class TestFluxControlPipelineMemory(FluxControlPipelineTesterConfig, MemoryTesterMixin): |
There was a problem hiding this comment.
Should we also have tests from LoraMemoryTesterMixin? The base Flux tests has these:
diffusers/tests/pipelines/flux/test_pipeline_flux.py
Lines 484 to 485 in 06e0f2a
There was a problem hiding this comment.
Control tests din't ever test those. So, I guess it's fine to not have it now.
|
|
||
| # This should be initialized with a Flux pipeline variant that doesn't accept `control_image`. | ||
| components["transformer"] = transformer | ||
| pipe = FluxPipeline(**components).to(torch_device) |
There was a problem hiding this comment.
nit: if I understand correctly, the components here will all be in train mode initially since get_dummy_components creates them in train mode, so pipe has all components in train mode. However, because get_pipeline sets all components to eval mode, control_pipe below has all components in eval mode. This creates a train vs eval mismatch between original_out and lora_out. Since hf-internal-testing/tiny-random-t5 uses "dropout_rate": 0.1, this means that original_out is computed with dropout but lora_out is not.
The test is still passes even with the mismatch, but should we explicitly set the components to eval mode here?
There was a problem hiding this comment.
I think it's fine because we're not asserting against a precomputed value slice.
| inputs = self.get_dummy_inputs(device) | ||
| image = pipe(**inputs).images | ||
| original_image_slice = image[0, -3:, -3:, -1] | ||
| image_slice = self.run_pipe(pipe)[0, -3:, -3:, -1] |
There was a problem hiding this comment.
Since we're now setting output_type to "pt" rather than "np", should we update the image_slice here? outpput_type="pt" creates a channels-first output (batch, channels, height, width) as noted in get_dummy_inputs, so we're taking a weird slice of the output currently.
There was a problem hiding this comment.
Yeah but we are not asserting against a precomputed slice here which is why the testing is doing fine. But I will change it for consistency standards, thanks!
2 participants
Following #14268