Deprecate flush_logs_every_n_steps on Trainer

[edward-io]

🚀 Feature

Deprecate flush_logs_every_n_steps from Trainer and make it available as a parameter to loggers that have this capability.

Motivation

We are auditing the Lightning components and APIs to assess opportunities for improvements:

• https://docs.google.com/document/d/1xHU7-iQSpp9KJTjI3As2EM0mfNHHr37WZYpDpwLkivA/edit#
• Review Lightning architecture & API #7740

Flushing should be considered an internal implementation detail of each logger. For example, TensorBoard automatically flush logs after a given amount of time (flush_secs).

Currently, flushing logs is configured through Trainer, which seems like the wrong level of abstraction. Setting flush_logs_every_n_steps given a TensorBoard logger doesn’t actually flush to disk, but calls log_metrics, which can be misleading to the user.

Prior issue: #4664

Pitch

Deprecate flush_logs_every_n_steps from Trainer, and move it to the init for logger classes that support this functionality (e.g. CSVLogger).
The logger connector already passes in the step information (self.trainer.logger.agg_and_log_metrics(scalar_metrics, step=step)) so we can move the flushing logic to a utility function.

Alternatives

Additional context

---

If you enjoy Lightning, check out our other projects! ⚡

• Metrics: Machine learning metrics for distributed, scalable PyTorch applications.

• Flash: The fastest way to get a Lightning baseline! A collection of tasks for fast prototyping, baselining, finetuning and solving problems with deep learning

• Bolts: Pretrained SOTA Deep Learning models, callbacks and more for research and production with PyTorch Lightning and PyTorch

• Lightning Transformers: Flexible interface for high performance research using SOTA Transformers leveraging Pytorch Lightning, Transformers, and Hydra.

[yopknopixx]

Hello! I would like to work on this. Please let me know how to proceed.

[ananthsub]

cc @tchaton @carmocca @awaelchli @justusschock

[awaelchli]

Agree with this one. Loggers behave very differently here, and it would require documenting which loggers are affected by this trainer argument and which are not.

[tchaton]

I agree with this one. However, I think we should provide some management within the BaseLogger to handle this logic and not introduce a BC in behaviour outside of depreciation @ananthsub.

[ananthsub]

I agree with this one. However, I think we should provide some management within the BaseLogger to handle this logic and not introduce a BC in behaviour outside of depreciation @ananthsub.

as @awaelchli points out, loggers behave very differently, so I think it would be challenging to add this to the base logger. for example, the tensorboard logger still wouldn't respect such a setting because the flushing logic is ultimately determined by its internal summary writer.

however, I think we could add buffering support for the relevant loggers that don't yet have this, but still a challenge here is the flushing interval might not be step based. it could also be time-based or even size-based