Standardize fast pipeline tests with PipelineTestMixin #1526

anton-l · 2022-12-02T17:05:18Z

Moves some of the common tests (save/load, output consistency, determinism) to a common PipelineTesterMixin
Adds dummy pipeline and inputs generators to pass to the mixin class and tidy up the rest of the tests.

HuggingFaceDocBuilderDev · 2022-12-02T17:09:26Z

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

tests/pipelines/stable_diffusion/test_stable_diffusion.py

patrickvonplaten · 2022-12-05T10:36:47Z

tests/test_pipelines_common.py

+        max_diff = np.abs(output - output_loaded).max()
+        self.assertLessEqual(max_diff, 1e-5)
+
+    def test_tuple_output(self):


Nice! That's very good for now ! In the future we could expand this test to also automatically make sure that not only the first tuple is the same but all the others now.

For now this would e.g. apply to the stable diffusion pipeline.

tests/test_pipelines_common.py

tests/pipelines/stable_diffusion/test_stable_diffusion.py

pcuenca

I like the approach in principle, I think it works to simplify and make tests coherent!

patrickvonplaten

Design looks nice to me! Just think we could add some more common tests. Note the more tests we add here the more "safety" we will get for free. Some additional tests we could add:

We can for now assume that every pipeline has a num_inference_steps parameter IMO. I'd also add a test that runs every pipeline with 2,3 different num_inference_steps and makes sure that a) output sizes are always the same, b) speed is less when less steps, ...
from_pretrained from the hub and locally -> IMO every pipeline should also define a parameter dummy_components_on_hub in addition to get_common_pipeline_components() this dummy components could then be used to check that loading from hub saving loading same locally gives same results
use dummy_components_on_hub and add a EXPECTED_SLICE to create a common test that makes sure that dummy weights on the Hub give expected results. This should help us to get rid of a lot of boiler plate fast tests that check for numerical equivalence.
The def components function can/should also be tested here. E.g. just check that components is the same as get_commen_pipeline_components and also the same as the init signature without the optional arguments.
enabling disabling the progress bar can be checked
Load/save with safetensors

=> Contrary to src/diffusers I'm really happy to go full abstraction mode here, so the more "common" tests we add here the faster we'll be able to add new pipelines going forward.

patrickvonplaten · 2022-12-05T11:02:33Z

Also, some signature tests would be nice e.g. for now we could maybe "force" every pipeline to have a num_inference_steps input

anton-l · 2022-12-06T11:51:44Z

Added the requested tests, except for:

from_pretrained from the hub + dummy_components_on_hub: will add those in a separate PR after mass-generating the missing checkpoints
EXPECTED_SLICE: haven't seen many repeated slices so far, but might revisit this later after updating the rest of the tests
Load/save with safetensors: we don't have save_pretrained with safetensors yet, but loading can probably be tested with the same dummy_components_on_hub where safetensors are in the same repo

anton-l · 2022-12-06T12:05:01Z

tests/test_pipelines_common.py

+        output_loaded = pipe_loaded(**inputs)[0]
+
+        max_diff = np.abs(output - output_loaded).max()
+        self.assertLess(max_diff, 3e-3, "The output of the fp16 pipeline changed after saving and loading.")


We might have an opportunity to make fp16 more stable somewhere, the error of about 0.002 even when running the same pipeline twice (without saving) seems suspicious to me

anton-l · 2022-12-06T15:39:22Z

tests/pipelines/altdiffusion/test_alt_diffusion.py

+        # TODO: address the non-deterministic text encoder (fails for save-load tests)
+        # torch.manual_seed(0)
+        # text_encoder_config = RobertaSeriesConfig(
+        #     hidden_size=32,
+        #     project_dim=32,
+        #     intermediate_size=37,
+        #     layer_norm_eps=1e-05,
+        #     num_attention_heads=4,
+        #     num_hidden_layers=5,
+        #     vocab_size=5002,
+        # )
+        # text_encoder = RobertaSeriesModelWithTransformation(text_encoder_config)
+
        torch.manual_seed(0)
-        config = RobertaSeriesConfig(
+        text_encoder_config = CLIPTextConfig(
+            bos_token_id=0,
+            eos_token_id=2,


@patil-suraj the AltDiffusion pipeline produces non-matching outputs if I run it with the same inputs twice. Replacing RobertaSeriesModelWithTransformation with CLIPTextModel helped, so it's probably that.

Hmmm weird, could you maybe open a seperate issue for this? I can look into it :-) We should test with RobertaSeriesConfig here IMO :-)

anton-l · 2022-12-06T16:23:51Z

tests/test_pipelines_common.py

+    def test_save_load_local(self):
+        if torch_device == "mps" and self.pipeline_class in (
+            DanceDiffusionPipeline,
+            CycleDiffusionPipeline,
+            StableDiffusionImg2ImgPipeline,
+        ):
+            # FIXME: inconsistent outputs on MPS
+            return


cc @pcuenca I might need your expertise with these, whenever you have time 😅

As discussed offline let's maybe open an issue here. But it's very nice that we spotted these bugs :-)

patrickvonplaten · 2022-12-06T16:59:13Z

tests/pipelines/karras_ve/test_karras_ve.py


 torch.backends.cuda.matmul.allow_tf32 = False


-class KarrasVePipelineFastTests(PipelineTesterMixin, unittest.TestCase):
+class KarrasVePipelineFastTests(unittest.TestCase):


Guess this whole class needs an API update no? Could we maybe also open an issue for this?

patrickvonplaten · 2022-12-06T17:06:03Z

tests/test_pipelines_common.py

+
+    @property
+    def pipeline_class(self) -> Union[Callable, DiffusionPipeline]:
+        raise NotImplementedError(


very nice! This forces new models to be nicely tested :-)

tests/test_pipelines_common.py

patrickvonplaten · 2022-12-06T17:07:32Z

tests/test_pipelines_common.py

+        max_diff = np.abs(output_with_offload - output_without_offload).max()
+        self.assertLess(max_diff, 1e-5, "XFormers attention should not affect the inference results")
+
+    def test_progress_bar(self):


patrickvonplaten · 2022-12-06T17:08:51Z

tests/test_pipelines_common.py

@@ -9,4 +27,347 @@ class PipelineTesterMixin:
    equivalence of dict and tuple outputs, etc.
    """

-    pass
+    # set these parameters to False in the child class if the pipeline does not support the corresponding functionality
+    test_attention_slicing = True


very cool to reuse transformers mechanism here!

patrickvonplaten

Great work! Let's merge it!

Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>

) * [WIP] Standardize fast pipeline tests with PipelineTestMixin * refactor the sd tests a bit * add more common tests * add xformers * add progressbar test * cleanup * upd fp16 * CycleDiffusionPipelineFastTests * DanceDiffusionPipelineFastTests * AltDiffusionPipelineFastTests * StableDiffusion2PipelineFastTests * StableDiffusion2InpaintPipelineFastTests * StableDiffusionImageVariationPipelineFastTests * StableDiffusionImg2ImgPipelineFastTests * StableDiffusionInpaintPipelineFastTests * remove unused mixins * quality * add missing inits * try to fix mps tests * fix mps tests * add mps warmups * skip for some pipelines * style * Update tests/test_pipelines_common.py Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com> Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>

[WIP] Standardize fast pipeline tests with PipelineTestMixin

38420b9

refactor the sd tests a bit

9349279

anton-l commented Dec 5, 2022

View reviewed changes

tests/pipelines/stable_diffusion/test_stable_diffusion.py Show resolved Hide resolved

patrickvonplaten reviewed Dec 5, 2022

View reviewed changes

tests/test_pipelines_common.py Outdated Show resolved Hide resolved

patrickvonplaten reviewed Dec 5, 2022

View reviewed changes

tests/test_pipelines_common.py Show resolved Hide resolved

patrickvonplaten reviewed Dec 5, 2022

View reviewed changes

tests/test_pipelines_common.py Outdated Show resolved Hide resolved

patrickvonplaten reviewed Dec 5, 2022

View reviewed changes

tests/test_pipelines_common.py Outdated Show resolved Hide resolved

patrickvonplaten reviewed Dec 5, 2022

View reviewed changes

tests/test_pipelines_common.py Show resolved Hide resolved

patrickvonplaten reviewed Dec 5, 2022

View reviewed changes

tests/pipelines/stable_diffusion/test_stable_diffusion.py Show resolved Hide resolved

pcuenca reviewed Dec 5, 2022

View reviewed changes

patrickvonplaten reviewed Dec 5, 2022

View reviewed changes

patrickvonplaten requested a review from williamberman December 5, 2022 10:59

anton-l added 3 commits December 5, 2022 15:33

add more common tests

f5ae7ec

add xformers

d898a24

add progressbar test

ce1b776

anton-l added 2 commits December 6, 2022 12:58

cleanup

26d7a3b

upd fp16

836fbe2

anton-l commented Dec 6, 2022

View reviewed changes

anton-l added 8 commits December 6, 2022 13:40

CycleDiffusionPipelineFastTests

148546f

DanceDiffusionPipelineFastTests

3e6a354

AltDiffusionPipelineFastTests

488a6b5

StableDiffusion2PipelineFastTests

cb61d0a

StableDiffusion2InpaintPipelineFastTests

75060ac

Merge main

c4baf7f

StableDiffusionImageVariationPipelineFastTests

4fb91fd

StableDiffusionImg2ImgPipelineFastTests

32d6b4b

anton-l added 4 commits December 6, 2022 16:19

StableDiffusionInpaintPipelineFastTests

4e8b51a

remove unused mixins

58b31b8

quality

806af67

add missing inits

a222de7

anton-l commented Dec 6, 2022

View reviewed changes

anton-l added 4 commits December 6, 2022 16:42

try to fix mps tests

8789df6

fix mps tests

24400cf

add mps warmups

efaf2fe

skip for some pipelines

2799c55

anton-l changed the title ~~[WIP] Standardize fast pipeline tests with PipelineTestMixin~~ Standardize fast pipeline tests with PipelineTestMixin Dec 6, 2022

anton-l marked this pull request as ready for review December 6, 2022 16:13

style

ee38f96

anton-l commented Dec 6, 2022

View reviewed changes

patrickvonplaten reviewed Dec 6, 2022

View reviewed changes

tests/test_pipelines_common.py Outdated Show resolved Hide resolved

patrickvonplaten reviewed Dec 6, 2022

View reviewed changes

patrickvonplaten approved these changes Dec 6, 2022

View reviewed changes

Update tests/test_pipelines_common.py

0800844

Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>

anton-l merged commit 02d83c9 into main Dec 6, 2022

anton-l deleted the pipeline-test-mixins branch December 6, 2022 17:35

anton-l mentioned this pull request Dec 6, 2022

[Maintenance] Updating fast pipeline tests to rely on common PipelineTesterMixin #1573

Open

31 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Standardize fast pipeline tests with PipelineTestMixin #1526

Standardize fast pipeline tests with PipelineTestMixin #1526

anton-l commented Dec 2, 2022 •

edited

HuggingFaceDocBuilderDev commented Dec 2, 2022 •

edited

patrickvonplaten Dec 5, 2022

pcuenca left a comment

patrickvonplaten left a comment

patrickvonplaten commented Dec 5, 2022

anton-l commented Dec 6, 2022

anton-l Dec 6, 2022

anton-l Dec 6, 2022

patrickvonplaten Dec 6, 2022

anton-l Dec 6, 2022

patrickvonplaten Dec 6, 2022

patrickvonplaten Dec 6, 2022

patrickvonplaten Dec 6, 2022

patrickvonplaten Dec 6, 2022

patrickvonplaten Dec 6, 2022

patrickvonplaten left a comment

Standardize fast pipeline tests with PipelineTestMixin #1526

Standardize fast pipeline tests with PipelineTestMixin #1526

Conversation

anton-l commented Dec 2, 2022 • edited

HuggingFaceDocBuilderDev commented Dec 2, 2022 • edited

Choose a reason for hiding this comment

pcuenca left a comment

Choose a reason for hiding this comment

patrickvonplaten left a comment

Choose a reason for hiding this comment

patrickvonplaten commented Dec 5, 2022

anton-l commented Dec 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickvonplaten left a comment

Choose a reason for hiding this comment

anton-l commented Dec 2, 2022 •

edited

HuggingFaceDocBuilderDev commented Dec 2, 2022 •

edited