Lightning-AI / Lightning-AI/pytorch-lightning
Deprecate passing a container to `self.save_hyperparameters()`
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 31.4k
- Forks
- 3.8k
- Avg merge
- 6d 7h
- Merged PRs (30d)
- 6
Description
## 🚀 Deprecation
As the title says, consider to deprecate passing a container (dict, DictConfig, etc.) to the `LightningModule.save_hyperparameters`.
### Motivation
See #10280 for a bit more context. There exists a confusion of how to save hyperparameters in a LightningModule or DataModule when the argument passed to the constructor is a container. We see this pattern:
```python
# A)
class Model(LightningModule):
def __init__(self, hparams):
super().__init__()
self.save_hyperparameters(hparams)
```
when it really should just be:
```python
# B)
class Model(LightningModule):
def __init__(self, hparams):
super().__init__()
self.save_hyperparameters()
# or
self.save_hyperparameters("hparams")
```
Both ways work just fine until you try to load the model from a checkpoint:
```python
# Fails with A), works with B)
model = Model.load_from_checkpoint("path")
```
The reason this fails is that `Model.load_from_checkpiont` will take the container saved in the checkpoint and pass the arguments in as `Model(**kwargs)`. But the signature of the model does not match in A) because the model is supposed to receive the container as input, not its contents.
**Q: Why do A) and B) both exist?**
A: A) is an artifact of the old way of saving hyperparameters, back in 1.0 the recommended way was to have a container like this and assign it as `self.hparams = hparams`. But `save_hyperparameters` was introduced to generalize this while trying to be as backward compatible as possible. Lots of feet have been shot.
**Q: Why can't we just special-case things so it works both ways?**
A: One can special-case this example here but it's easy to find another example where things break down (just add more args). This has happened in the past. The new (B) way of saving hyperparams is the recomended way because IT JUST WORKS.
### Pitch
Deprecate passing a full container to the `save_hyperparameters()` method. A warning will be shown and give simple instructions to the user how they can change their code in one line.
### Alternatives
Still support it. Document it only in the API signature but not in our user guides.
### Additional context
The docs are being improved over here already: #10280
Closes also #8948
cc @borda @tchaton @rohitgr7 @akihironitta @carmocca
______________________________________________________________________
#### If you enjoy Lightning, check out our other projects! ⚡
- [**Metrics**](https://github.com/PyTorchLightning/metrics): Machine learning metrics for distributed, scalable PyTorch applications.
- [**Lite**](https://pytorch-lightning.readthedocs.io/en/latest/starter/lightning_lite.html): enables pure PyTorch users to scale their existing code on any kind of device while retaining full control over their own loops and optimization logic.
- [**Flash**](https://github.com/PyTorchLightning/lightning-flash): The fastest way to get a Lightning baseline! A collection of tasks for fast prototyping, baselining, fine-tuning, and solving problems with deep learning.
- [**Bolts**](https://github.com/PyTorchLightning/lightning-bolts): Pretrained SOTA Deep Learning models, callbacks, and more for research and production with PyTorch Lightning and PyTorch.
- [**Lightning Transformers**](https://github.com/PyTorchLightning/lightning-transformers): Flexible interface for high-performance research using SOTA Transformers leveraging Pytorch Lightning, Transformers, and Hydra.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at LightningModule.save_hyperparameters and Model.load_from_checkpoint, then trace how container arguments are saved and restored. Review the context in #10280 and #8948 before deciding the deprecation behavior; done means the legacy call warns with migration guidance while the recommended form continues to load checkpoints correctly.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, pytorch
- Domain
- backend-api-design, machine-learning
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100