search

huggingface / accelerate

🚀 A simple way to launch, train, and use PyTorch models on almost any device and distributed configuration, automatic mixed precision (including fp8), and easy-to-configure FSDP and DeepSpeed support
https://huggingface.co/docs/accelerate
Apache License 2.0
7.97k stars 970 forks source link

🚀 [Feature] Add deprecated Decorator #3161

Open yhna940 opened 1 month ago

yhna940 commented 1 month ago

What does this PR do?

Summary

This PR introduces a deprecated decorator, designed to mark functions as deprecated with detailed warnings, following the approach in torch/onnx/_deprecation.py. The decorator provides a standard way to issue deprecation warnings and update docstrings, informing users of the version when a function was deprecated, its removal target, and recommended alternative actions.

Changes

Example Usage

After this PR, the following usage:

from accelerate.utils.modeling import FindTiedParametersResult
results = FindTiedParametersResult([[1, 2, 3]])
results.values()

will produce output similar to:

<stdin>:1: FutureWarning: 'accelerate.utils.modeling.values' is deprecated in version 1.0.0rc0 and will be removed in 1.3.0. Please use another method instead.
[2, 3]

Testing

Included a test for deprecated to validate:

BenjaminBossan commented 1 month ago

Maybe not relevant, but Python added its own warnings.deprecated decorator in Python 3.13. Using the same name could be confusing. The Python decorator apparently also allows type checkers to flag the usage, which would be nice to have. Of course, we can't use it directly since we want to support older Python versions.

yhna940 commented 1 month ago

Maybe not relevant, but Python added its own warnings.deprecated decorator in Python 3.13. Using the same name could be confusing. The Python decorator apparently also allows type checkers to flag the usage, which would be nice to have. Of course, we can't use it directly since we want to support older Python versions.

Hello @BenjaminBossan, thank you for the insightful review! I wasn’t aware of the warnings.deprecated decorator introduced in Python 3.13, and I appreciate you pointing it out. As I am currently working with a lower Python version (likely 3.8, according to the Dev Container), I implemented a custom decorator to ensure compatibility.

That said, the name overlap could indeed be confusing. Would using an alternative name for the decorator help clarify things? I’d be glad to make adjustments if needed!

BenjaminBossan commented 1 month ago

As to the Python version, 3.8 is going to be end of life by the end of the month, so the containers will be updated soon. Still, it will be quite some time before 3.13 will be the lowest supported Python version, so we can't assume that the decorator exists. More importantly, I wonder if we can "borrow" some of the implementation details of warnings.deprecated, as I'm sure the Python devs put a lot of thought into it. Regarding the naming, I'll let Zach decide on that.

yhna940 commented 3 weeks ago

Thank you for your feedback, I agree with your suggestions regarding naming and compatibility! Using Python’s warnings.deprecated decorator in 3.13 and above, while defaulting to our custom decorator in lower versions, sounds like a balanced approach. This way, we maintain compatibility across versions while taking advantage of the built-in decorator when possible.

I’ve resolved the conflicts in the code for now, and I’ll be sure to apply any further adjustments based on additional feedback. Thanks again!

HuggingFaceDocBuilderDev commented 3 weeks ago

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.

matthewdouglas commented 2 weeks ago

Hi folks,

Could we consider using the implementation from typing_extensions>=4.5 instead? It could maybe be wrapped still to provide help with formatting for UX/docs, but that way we get improved IDE support and alignment with PEP 702.

BenjaminBossan commented 2 weeks ago

@matthewdouglas AFAICT, that implementation is a backport of warnings.deprecated that we discussed above. I agree that typing support would be nice to have, but feature-wise, the two are not on par, so not sure if we can simply replace it.

yhna940 commented 6 hours ago

I hope you're all doing well! I wanted to follow up on this PR to check if there are any remaining concerns or additional feedback that I should address to move it forward. Please let me know if there's anything else I can do to improve this PR :) Thank you for your time and guidance!