Lightning-AI / pytorch-lightning

Pretrain, finetune ANY AI model of ANY size on multiple GPUs, TPUs with zero code changes.
https://lightning.ai
Apache License 2.0
28.47k stars 3.39k forks source link

Use `FutureWarning` instead of `DeprecationWarning` for deprecation warning #11525

Open akihironitta opened 2 years ago

akihironitta commented 2 years ago

basically a copy of https://github.com/PyTorchLightning/metrics/issues/744 which was addressed in https://github.com/PyTorchLightning/metrics/pull/749


šŸš€ Feature

see suggestion in https://github.com/PyTorchLightning/metrics/pull/740#discussion_r782088021

Motivation

most of the deprecations in TM are meant to users not developers

Pitch

Replace DeprecationWarning with FutureWarning defined at: https://github.com/PyTorchLightning/pytorch-lightning/blob/033dba1494a177954e8ca59bc74b1635e83b9efa/pytorch_lightning/utilities/warnings.py#L44 and remove: https://github.com/PyTorchLightning/pytorch-lightning/blob/033dba1494a177954e8ca59bc74b1635e83b9efa/pytorch_lightning/utilities/warnings.py#L48-L49

Alternatives

Keep using DeprecationWarning.

Additional context


If you enjoy Lightning, check out our other projects! āš”

cc @borda

puhuk commented 2 years ago

Can I take this

edward-io commented 2 years ago

edit: added this as a comment to the PR directly.

Can we subclass FutureWarning to create a LightningFutureWarning class? We use the base class internally to log user deprecations, e.g.

class LightningFutureWarning(FutureWarning):
  def __init__(self, ...):
     super().__init_(...)
     log_event(UserDeprecationEvent(msg))

We'll be able to add these logging events to PyTorch Lightning OSS soon, torch.monitor is going to be released in the next version of PyTorch.

akihironitta commented 2 years ago

@edward-io Sounds good to me. @carmocca Shall we?

carmocca commented 2 years ago

@edward-io Why not add that log_event call to rank_zero_deprecation instead?

carmocca commented 2 years ago

One big downside of using FutureWarning is that we would need to replace all pytest.deprecated_call uses. https://github.com/PyTorchLightning/pytorch-lightning/pull/11527 is currently failing for this reason.

ananthsub commented 2 years ago

One big downside of using FutureWarning is that we would need to replace all pytest.deprecated_call uses. https://github.com/PyTorchLightning/pytorch-lightning/pull/11527 is currently failing for this reason.

Would we need to find/replace these with pytest.warns(FutureWarning) ?