Lightning-AI / Lightning-AI/pytorch-lightning
[RFC] Logger_connector should own the logger, and should not have a Trainer reference
@daniellepintz is already working on this.
Since Mar 2, 2022.
- Dominant language
- Python
- Stars
- 31.4k
- Forks
- 3.8k
- Avg merge
- 6d 7h
- Merged PRs (30d)
- 6
Description
## Proposed refactor
* Create a `logger` property on the logger_connector
* Create a read-only property on the Trainer called `logger` which returns `logger_connector.logger`
* Remove Trainer reference from logger_connector
### Motivation
* Currently the Trainer owns the logger, but it is configured/set in the logger_connector:
https://github.com/PyTorchLightning/pytorch-lightning/blob/32e7d32956e1685d36f2ab0ca3770baa2f76ce10/pytorch_lightning/trainer/connectors/logger_connector/logger_connector.py#L78-L91
* The logger should be written to in the same class where it is owned
* The Trainer class is already giant and does a lot, so let's have the logger_connector own all things related to the loggers, including the loggers themselves. (We will still have a read-only property for the loggers on the Trainer)
* This approach is similar to what we do for the accelerator_connector. The accelerator_connector owns the accelerator related properties, and has no reference to the Trainer.
Part of #10417
### Pitch
* Create a `logger` property on the logger_connector
* Update all the reference of `trainer.logger` on the logger_connector to `self.logger`
* Create a read-only property on the Trainer called `logger` which returns `logger_connector.logger`
* Update all the other places where the trainer is used in logger_connector - some variables will have to be passed in the constructor, and others will have to be passed as arguments to some methods such as `should_flush_logs`
* Remove Trainer reference from logger_connector
cc @justusschock @awaelchli @akihironitta @rohitgr7 @edward-io @borda @ananthsub @kamil-kaczmarek @Raalsky @Blaizzy @kaushikb11 @ninginthecloud
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.
Assessment
This issue has not been assessed yet.