Lightning-AI / Lightning-AI/pytorch-lightning
[RFC] Clean up Logger saving of Profiler output
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 31.4k
- Forks
- 3.8k
- Avg merge
- 6d 7h
- Merged PRs (30d)
- 6
Description
### Background
Currently we write our checkpoint and profiler output to the Logger, in the case that the user does not specify a path.
(Note: this is for the case of one logger, the case of multiple loggers to be discussed in a different issue).
The exact priority followed is:
Profiler:
1. if a `dirpath` is provided write to it
2. if not, write to the Logger
3. if there is no Logger, write to `default_root_dir`
Checkpointing:
1. if a `dirpath` is provided write to it
2. if not, and a `weights_save_path` is provided in the Trainer, write to it (Deprecated in https://github.com/PyTorchLightning/pytorch-lightning/issues/11768)
3. if not, write to the Logger
4. if there is no Logger, write to `default_root_dir`
### Motivation
There are several issues and inconsistencies with how we currently write to the Logger.
1. One issue is that for the checkpoint output we put it in a nice directory called "checkpoints", but for the profiler output it is not in a nice directory called "profiler" but instead the individual output files are just sitting in one of the logger's directories.

2. There are inconsistencies in terms of where the loggers store the profiler output (in the case where no dirpath is provided). Most of the loggers store the output in their `save_dir`, but the TensorBoardLogger stores the profiler output in the same place where it stores the checkpoints - `save_dir/name/version`.
https://github.com/PyTorchLightning/pytorch-lightning/blob/e15a66412cc220fa241ec7cbb64b339a2f124761/pytorch_lightning/trainer/trainer.py#L2095-L2107
Why do we have different behavior for the TensorBoardLogger? **Also, why is the profiler output not in the same place as the checkpointing output?**
### Pitch
1. When logging to the logger, put the profiler output in a directory called "profiler".
2. All loggers should store their output in `save_dir/name/version`. Introduce a new property on the Logger Base API called `log_dir` (TBLogger and CSVLogger already have it), which returns this path, and is the place to store profiler and checkpointing output.
This will allow us to clean up this code here:
https://github.com/PyTorchLightning/pytorch-lightning/blob/e15a66412cc220fa241ec7cbb64b339a2f124761/pytorch_lightning/trainer/trainer.py#L2095-L2107
And also massively simplify this code:
https://github.com/PyTorchLightning/pytorch-lightning/blob/e15a66412cc220fa241ec7cbb64b339a2f124761/pytorch_lightning/callbacks/model_checkpoint.py#L582-L596
replacing it with simply `ckpt_path = os.path.join(trainer.log_dir, "checkpoints")` after https://github.com/PyTorchLightning/pytorch-lightning/issues/11768 is also complete.
cc @awaelchli @edward-io @borda @ananthsub @rohitgr7 @kamil-kaczmarek @Raalsky @Blaizzy @ninginthecloud @carmocca @kaushikb11
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 by reading the Logger Base API and the existing log_dir implementations in TBLogger and CSVLogger. Then inspect trainer.py around the profiler path handling and callbacks/model_checkpoint.py around checkpoint path selection. Done means the proposed logger directory behavior is implemented consistently for profiler and checkpoint output, with the affected tests updated.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, pytorch
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100