[Utils] Add deprecate function and move testing_utils under utils #659

patrickvonplaten · 2022-09-27T20:06:18Z

Improved deprecation functionality

We are introducing multiple deprecation cycles already and currently are missing structure on how to handle this exactly and by adding **kwargs to the scheduler config init we have lost functionality. More specifically:

1. We do not keep track when functionality should be removed as we forget to remove the deprecation cycle and the functionality (this happened twice now)
1. Right now when you pass a incorrect argument, e.g. PNDMScheduler(num_inference_steps=5) nothing happens because it's swallowed by **kwargs. However we should, just as before [Pytorch] add dep. warning for pytorch schedulers #651 throw a TypeError in this case.

=> This PR solves this by introducing a deprecate function (which should be mainly used by maintainers) that automatically forces us to remove deprecated functionality correctly + solves 2. by throwing a type error. In addition it removes boiler plate code and should make it more efficient for us to deprecate things.

The API of deprecate is as follows:

new_arg = deprecate("old_arg", "0.7.0", "Please use `new_arg` instead", take_from=kwargs) or new_arg

=> This forces us to remove this statement when version >= 0.7.0, gives a nice default error message and makes sure that kwargs can only have deprecated args.

Please take a look into the changes here and the tests to better understand all the functionality

HuggingFaceDocBuilderDev · 2022-09-27T20:55:54Z

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

…add_deprecate_function

patrickvonplaten · 2022-09-29T16:51:10Z

src/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion.py

-                DeprecationWarning,
+                " file"
            )
+            deprecate("steps_offset!=1", "1.0.0", deprecation_message, standard_warn=False)


This is a big change and I like to keep it for a bit until we force people to not pass steps_offset anymore.

patrickvonplaten · 2022-09-29T16:51:40Z

src/diffusers/schedulers/scheduling_ddim.py

-
-            offset = kwargs["offset"]
+        deprecated_offset = deprecate(
+            "offset", "0.5.0", "Please pass `steps_offset` to `__init__` instead.", take_from=kwargs


bump as we would have already missed this version

patrickvonplaten · 2022-09-29T16:51:52Z

src/diffusers/schedulers/scheduling_pndm.py

-
-            offset = kwargs["offset"]
+        deprecated_offset = deprecate(
+            "offset", "0.5.0", "Please pass `steps_offset` to `__init__` instead.", take_from=kwargs


bump as we would have already missed this version

patrickvonplaten · 2022-09-29T16:52:11Z

src/diffusers/utils/outputs.py

-                    " `'images'` instead.",
-                    DeprecationWarning,
-                )
+                deprecate("samples", "0.6.0", "Please use `.images` or `'images'` instead.")


bump twice as we would have already missed this version and it's important

patrickvonplaten · 2022-09-29T16:52:31Z

tests/test_utils.py

+from diffusers.utils import deprecate
+
+
+class DeprecateTester(unittest.TestCase):


This test best describes all the functionality that we need to ensure

patil-suraj

Thanks a lot for working on this, this will be very useful!
And really nice tests 😍 ! Love this way of explaining new features with tests.

patil-suraj · 2022-09-30T10:23:19Z

src/diffusers/utils/testing_utils.py

+def deprecate(*args, take_from: Optional[Union[Dict, Any]] = None, standard_warn=True):
+    from .. import __version__
+
+    deprecated_kwargs = take_from
+    values = ()
+    if not isinstance(args[0], tuple):
+        args = (args,)
+
+    for attribute, version_name, message in args:
+        if version.parse(version.parse(__version__).base_version) >= version.parse(version_name):
+            raise ValueError(
+                f"The deprecation tuple {(attribute, version_name, message)} should be removed since diffusers'"
+                f" version {__version__} is >= {version_name}"
+            )
+
+        warning = None
+        if isinstance(deprecated_kwargs, dict) and attribute in deprecated_kwargs:
+            values += (deprecated_kwargs.pop(attribute),)
+            warning = f"The `{attribute}` argument is deprecated and will be removed in version {version_name}."
+        elif hasattr(deprecated_kwargs, attribute):
+            values += (getattr(deprecated_kwargs, attribute),)
+            warning = f"The `{attribute}` attribute is deprecated and will be removed in version {version_name}."
+        elif deprecated_kwargs is None:
+            warning = f"`{attribute}` is deprecated and will be removed in version {version_name}."
+
+        if warning is not None:
+            warning = warning + " " if standard_warn else ""
+            warnings.warn(warning + message, DeprecationWarning)
+
+    if isinstance(deprecated_kwargs, dict) and len(deprecated_kwargs) > 0:
+        call_frame = inspect.getouterframes(inspect.currentframe())[1]
+        filename = call_frame.filename
+        line_number = call_frame.lineno
+        function = call_frame.function
+        key, value = next(iter(deprecated_kwargs.items()))
+        raise TypeError(f"{function} in {filename} line {line_number-1} got an unexpected keyword argument `{key}`")
+
+    if len(values) == 0:
+        return


pcuenca

This is extremely cool, and so useful! It was very helpful to start by reading the tests.

Seeing that deprecate was inside testing_utils.py sounded a bit off, so I looked more closely to see what was there and realized that we would be breaking import diffusers when PyTorch is not installed. In any case, I believe that the semantics of the function call for moving it to another place.

pcuenca · 2022-09-30T11:58:27Z

src/diffusers/schedulers/scheduling_pndm.py

+        deprecated_offset = deprecate(
+            "offset", "0.5.0", "Please pass `steps_offset` to `__init__` instead.", take_from=kwargs
+        )
+        offset = deprecated_offset or self.config.steps_offset


Great naming here, the intention is very clear!

pcuenca · 2022-09-30T11:59:34Z

src/diffusers/utils/testing_utils.py

+def deprecate(*args, take_from: Optional[Union[Dict, Any]] = None, standard_warn=True):
+    from .. import __version__
+
+    deprecated_kwargs = take_from
+    values = ()
+    if not isinstance(args[0], tuple):
+        args = (args,)
+
+    for attribute, version_name, message in args:
+        if version.parse(version.parse(__version__).base_version) >= version.parse(version_name):
+            raise ValueError(
+                f"The deprecation tuple {(attribute, version_name, message)} should be removed since diffusers'"
+                f" version {__version__} is >= {version_name}"
+            )
+
+        warning = None
+        if isinstance(deprecated_kwargs, dict) and attribute in deprecated_kwargs:
+            values += (deprecated_kwargs.pop(attribute),)
+            warning = f"The `{attribute}` argument is deprecated and will be removed in version {version_name}."
+        elif hasattr(deprecated_kwargs, attribute):
+            values += (getattr(deprecated_kwargs, attribute),)
+            warning = f"The `{attribute}` attribute is deprecated and will be removed in version {version_name}."
+        elif deprecated_kwargs is None:
+            warning = f"`{attribute}` is deprecated and will be removed in version {version_name}."
+
+        if warning is not None:
+            warning = warning + " " if standard_warn else ""
+            warnings.warn(warning + message, DeprecationWarning)
+
+    if isinstance(deprecated_kwargs, dict) and len(deprecated_kwargs) > 0:
+        call_frame = inspect.getouterframes(inspect.currentframe())[1]
+        filename = call_frame.filename
+        line_number = call_frame.lineno
+        function = call_frame.function
+        key, value = next(iter(deprecated_kwargs.items()))
+        raise TypeError(f"{function} in {filename} line {line_number-1} got an unexpected keyword argument `{key}`")
+
+    if len(values) == 0:
+        return


pcuenca · 2022-09-30T12:09:47Z

src/diffusers/utils/testing_utils.py

-from typing import Union
+from typing import Any, Dict, Optional, Union

 import torch


I think this breaks import diffusers if PyTorch is not installed.

We should move deprecate to its own file. Not sure whether we should call it deprecate.py, deprecation_utils.py or something more general in case we need more functionality in the future.

Sounds good! I'll add it to deprecation_utils.py

Regarding import torch, this should be done in another PR - this is unrelated to this PR (import torch was there before). We should definitely make sure though that the library doesn't break when torch is not installed.

Regarding import torch, this should be done in another PR - this is unrelated to this PR (import torch was there before).

Yes, but testing_utils was only imported when testing, that's really the difference (not the import). I should have made it clearer in my comment :)

…ggingface#659) * [Utils] Add deprecate function * up * up * uP * up * up * up * up * uP * up * fix * up * move to deprecation utils file * fix * fix * fix more

…ace#659) (This goes on to produce compilation errors, but one step at a time)

…ggingface#659) * [Utils] Add deprecate function * up * up * uP * up * up * up * up * uP * up * fix * up * move to deprecation utils file * fix * fix * fix more

patrickvonplaten added 4 commits September 27, 2022 22:06

[Utils] Add deprecate function

6db4338

up

8447d88

up

5cde36b

uP

2d80a08

patrickvonplaten added 6 commits September 28, 2022 10:45

up

e1af3ab

up

04909f8

up

e51a656

Merge branch 'main' of https://github.com/huggingface/diffusers into …

3b2d8af

…add_deprecate_function

up

dec5930

uP

812fb8c

patrickvonplaten changed the title ~~[Utils] Add deprecate function~~ [Utils] Add deprecate function and move testing_utils under utils Sep 29, 2022

patrickvonplaten commented Sep 29, 2022

View reviewed changes

patrickvonplaten added 3 commits September 29, 2022 18:58

up

7a8eb4a

fix

e6187f4

up

43f984c

patrickvonplaten requested review from anton-l, patil-suraj and pcuenca September 29, 2022 17:04

patil-suraj approved these changes Sep 30, 2022

View reviewed changes

pcuenca reviewed Sep 30, 2022

View reviewed changes

pcuenca mentioned this pull request Sep 30, 2022

Can't import diffusers with Flax but without PyTorch #685

Closed

patrickvonplaten added 4 commits October 3, 2022 23:03

Merge from main

82c3abb

move to deprecation utils file

99e8480

fix

a9d9ba1

fix

b6ae0d0

fix more

4db2c92

patrickvonplaten merged commit f1484b8 into main Oct 3, 2022

patrickvonplaten deleted the add_deprecate_function branch October 3, 2022 21:44

patrickvonplaten mentioned this pull request Oct 6, 2022

Add back-compatibility to LMS timesteps #750

Merged

PhaneeshB pushed a commit to nod-ai/diffusers that referenced this pull request Mar 1, 2023

Fix NoneType exception if vulkan tuning flags not detected. (huggingf…

523e63f

…ace#659) (This goes on to produce compilation errors, but one step at a time)

		from diffusers.utils import deprecate


		class DeprecateTester(unittest.TestCase):

[Utils] Add deprecate function and move testing_utils under utils #659

[Utils] Add deprecate function and move testing_utils under utils #659

Uh oh!

Conversation

patrickvonplaten commented Sep 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Improved deprecation functionality

Uh oh!

HuggingFaceDocBuilderDev commented Sep 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patil-suraj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pcuenca left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

patrickvonplaten commented Sep 27, 2022 •

edited

Loading

HuggingFaceDocBuilderDev commented Sep 27, 2022 •

edited

Loading