Pytorch-lightning: logging basic configuration level: INFO vs. WARNING (usability with W&B)

[colehurwitz]

Thanks for the amazing package! I am having a great time using it.

Issue

Recently, I have been playing around with the weights and biases (W&B) logging functionality and I noticed that I was getting a lot of logging messages in my jupyter notebook while training my model (every epoch I got new messages).

When I looked into logging.__init__.py, the logging basic configuration was set with:

logging.basicConfig(level=logging.INFO)

However, that means a lot of INFO messages are being printed from W&B's use of the logger (e.g. when the RunManager modifies files).

Potential Solution

To disable this, I switched the logging basic configuration in logging.__init__.py to:

logging.basicConfig(level=logging.WARNING)

Would this be a useful addition in general to the package or should I just keep this change local?

[neggert]

Honestly, best practice is for libraries not to configure logging at all. That way the user can set it up however they want. When libraries configure logging, you get weird things like this.

logging.basicConfig(level=logging.DEBUG)
import pytorch_lightning.logging
# now log level is INFO :(

@williamFalcon thoughts on leaving logging configuration to the users?

[Borda]

the issue with logging is that it has to be sett by logging.basicConfig before fist usage, another option is to get the logger during a run and change the level... or we may think about using another logging package... let's have look at https://github.com/Delgan/loguru

[williamFalcon]

@neggert i agree that we should leave up to user but this is something people will have to remember... so maybe we set a default one if there's no config already set @Borda ?

[Borda]

I will need to check it...

[neggert]

Okay, spent a little bit of time thinking about this. I think it's fine to setup the lightning logger for the user, but we shouldn't be touching other loggers (e.g. WandB). To give a little more visibility to the user, what do we think about adding another arg to Trainer?

Arg:
    log_level (str, optional): Log level for the pytorch_lightning logger. Should be one of
    "DEBUG", "INFO", "WARNING", "ERROR", or `None`. If set to `None`, no logging
    configuration will be done; the user must configure logging if they want something
    other than Python defaults. Defaults to "INFO".

Then in the code, we do something like

if log_level is not None:
    logging.getLogger("pytorch_lighting").setLevel(log_level)

@Borda @williamFalcon Any thoughts? Only downside I see is yet another argument to Trainer.

[cmpute]

I think the best practice is actually letting the package use dedicated logger, just as @neggert wrote: use logging.getLogger("pytorch_lightning"). Messing up with root logger has created much trouble in my case...

[cmpute]

It's fine if default level of dedicated logger is set, since it won't change the level of other loggers, while users can still change the logger level if they want

And also the loggers under /loggers package should use subloggers (like logging.getLogger('pytorch_lightning.neptune') or maybe shorter like logging.getLogger('lightning.neptune'). Thus the log_level can be inherited.

[Borda]

yeah, having different logging level per logger sounds good... on the other hand, experiment-like logger does not save any logging messages aka terminal...
We can freely remove setting logging level from package init I guess. just trying to remember what was the reason we place it there...
I think that the case was that the logging was used somewhere in lightning package in plane code (not inside a function) and then touching this logging without proper setting basicConfing caused that no logging messages were shown...

[Borda]

And also the loggers under /loggers package should use subloggers (like logging.getLogger('pytorch_lightning.neptune') or maybe shorter like logging.getLogger('lightning.neptune'). Thus the log_level can be inherited.

since #825 we support multiple loggers... The common practice is to have a WARNING level in a terminal and DEBUG in a file... What we could do is add a logger which would catch all logging messages like info/debug and write them to a special file... @cmpute is it something you are interested in?

[cmpute]

And also the loggers under /loggers package should use subloggers (like logging.getLogger('pytorch_lightning.neptune') or maybe shorter like logging.getLogger('lightning.neptune'). Thus the log_level can be inherited.

since #825 we support multiple loggers... The common practice is to have a WARNING level in a terminal and DEBUG in a file... What we could do is add a logger which would catch all logging messages like info/debug and write them to a special file... @cmpute is it something you are interested in?

By "logging" I mean maybe the text logging (with logging python module) should go under certain namespace instead of using root logger, for better text logging management

[Borda]

Sounds interesting, @cmpute mind submit PR with your suggestion?

[cmpute]

@Borda sure, I can have a try