Skip to content

Length of loggers #17280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bipin-lekhak opened this issue Apr 5, 2023 · 2 comments
Closed

Length of loggers #17280

bipin-lekhak opened this issue Apr 5, 2023 · 2 comments
Labels
feature Is an improvement or enhancement logger Related to the Loggers

Comments

@bipin-lekhak
Copy link

bipin-lekhak commented Apr 5, 2023

Description & Motivation

The feature to use multiple loggers is really helpful. But I have issues when I try to handle code in such a way that any logger maybe turned off at any time.

For example, when I'm writing this code, I am writing a model able to log into mlflow and tensorboard. When both loggers are passed to Trainer(logger=[ml_flow, tesnorboard_logger]) vs Trainer(logger=[tesnorboard_logger]), I have to write a lot of different custom code to handle that. Upstream handling of how many loggers were passed and of which type in which order is messy. Example, I want to log an image to tesnorboard logs when I only pass tensorboard logger will be: self.logger.experiment.add_image() but when multiple logs are passed and in unknown order, the code becomes unnecessarily convoluted.

Pitch

Handle multiple loggers in model easily. We should be able to check if logger of a type is present or not and if present, get them. This should happen with minimal friction such as checking all indexes. One solution may be to have self.loggers as a dictionary rather than list?

Alternatives

Providing __len__ for pytorch_lightning.loggers.base.LoggerCollection can be one low hanging fruit which does simplify a lot (of course doesnot go all the way).

Additional context

No response

cc @Borda @awaelchli @Blaizzy

@bipin-lekhak bipin-lekhak added feature Is an improvement or enhancement needs triage Waiting to be triaged by maintainers labels Apr 5, 2023
@awaelchli
Copy link
Contributor

Hi @bipin-lekhak
Sorry for the delayed response.

We actually got rid of the LoggerCollection and opted for a simpler design where loggers are just exposed in a list. As of Lightning 1.8.0, trainer.logger returns the first logger in the list and trainer.loggers returns the list of all loggers. Our recommendation for users who have logger-specific code is to implement their calls like so:

for logger in trainer.loggers:
    if isinstance(logger, WandbLogger)
        logger.experiment.wandb_api_call()
    if isinstance(logger, MLflowLogger)
        logger.experiment.mlflow_api_call()

This is only needed if users need to make logger-api-specific calls. For regular metrics, self.log() in the LightningModule remains logger-agnostic.

To your feature request:
If it helps, you can check len(trainer.loggers) today.

LoggerCollection was never able to solve this problem of logger-specific api calls. Plus, we tried to unify logger apis in the past but were never successful (#12183, #11837).

Relevant issues:
#14283

Closing this for now.

@awaelchli awaelchli closed this as not planned Won't fix, can't repro, duplicate, stale Jun 12, 2023
@awaelchli awaelchli added logger Related to the Loggers and removed needs triage Waiting to be triaged by maintainers labels Jun 12, 2023
@bipin-lekhak
Copy link
Author

Thanks for your response. My solution was something similar to what you have suggested. Your response validated my solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement logger Related to the Loggers
Projects
None yet
Development

No branches or pull requests

2 participants