Deprecate save from the Logger Base API

[daniellepintz]

Proposed refactor

Deprecate save from LightningLoggerBase:
https://github.com/PyTorchLightning/pytorch-lightning/blob/a8ee5cacb7b31064c8b73372ca944d43f7caadc6/pytorch_lightning/loggers/base.py#L173-L175

Motivation

After #9145 save on the LightningLoggerBase does nothing, and only 2 loggers have implementations for it: CSVLogger and TensorboardLogger.

After #8991 we now consider flushing an internal implementation detail of the loggers, so it follows that save should also be an internal implementation detail of the loggers.

Currently the Trainer calls log_metrics and save together here:
https://github.com/PyTorchLightning/pytorch-lightning/blob/6bc0e1da6329c42ec3b7c6fbf91eb059b9124207/pytorch_lightning/trainer/connectors/logger_connector/logger_connector.py#L131-L132

Instead, the Trainer can just call log_metrics and the actual saving can be delegated completely to the logger, including buffering & flush frequency.

Pitch

in v1.6:

• Deprecate save from LightningLoggerBase
• For the following three save calls, only call if save is overridden in the Logger:
  https://github.com/PyTorchLightning/pytorch-lightning/blob/6bc0e1da6329c42ec3b7c6fbf91eb059b9124207/pytorch_lightning/trainer/connectors/logger_connector/logger_connector.py#L132
  https://github.com/PyTorchLightning/pytorch-lightning/blob/6bc0e1da6329c42ec3b7c6fbf91eb059b9124207/pytorch_lightning/trainer/trainer.py#L1241
  https://github.com/PyTorchLightning/pytorch-lightning/blob/6bc0e1da6329c42ec3b7c6fbf91eb059b9124207/pytorch_lightning/loops/epoch/training_epoch_loop.py#L508
  (Note: the final save call above will be removed in v1.7 following the removal of flush_logs_every_n_steps)

in v1.8:

• For CSVLogger and TensorboardLogger their log_metrics and log_graph implementations should call save
• Remove save from LightningLoggerBase

Additional Context

This was discussed in #9004

cc @justusschock @awaelchli @akihironitta @rohitgr7 @edward-io @Borda @ananthsub @kamil-kaczmarek @Raalsky @Blaizzy @tchaton

[ananthsub]

Removing save from LightningLoggerBase makes sense to do only after v1.6 is released because flush_logs_every_n_steps will be removed in v1.7.
So the deprecation of save and removal of flush_logs_every_n_steps would happen in v1.7, with save targeted to be removed in v1.9.

[daniellepintz]

Hmm, sorry I'm not quite following why we would have to wait until flush_logs_every_n_steps is removed?

[daniellepintz]

@awaelchli @carmocca We didn't have time to finish discussing this one the other day but I just added more to the Motivation section. Please take a look when you get the chance.

[carmocca]

Looks good. One note though

Move the code currently in save for LightningLoggerBase, CSV, and Tensorboard to log_metrics

Instead, I think the subclasses should keep the save method and log_metrics should call self.save().

[daniellepintz]

@carmocca sounds good! Updated the pitch