Standardize API for logging images, histograms, etc.

[sharuevds]

🚀 Feature

Standardized way to log non-scalar data

Motivation

Different logging backends supported by PL use different ways to log such data, so it would be very convenient to have standardized way to do it. I mentioned it in Slack and got approval to make a PR, so I want to discuss details here.

Pitch

Add methods that allow to log images, histograms or other data (audio, video, ...) to PL logger class.

Additional context

What is better?

If the function is called, but current logger backend doesn't support this type of content:

• silently ignore: we misleading the user allowing him to use method that does nothing without any notification
• throw an error: it can be undesired behavior if LoggerCollection is used
• show a warning: IMO best solution

What signature should methods have?

• log_images(images: Dict[str, image_types]): consistent with existing log_metrics, but new method should be added for each type of data (log_images, log_histograms, ...); IMO best solution
• log_image(tag, image): consistent with Tensorboard, but can be inconvenient when logging multiple images
• log_metrics where image wrapper is passed as value: consistent with WandB (uses its own wandb.Image wrapper), but requires user to create wrappers and increases complexity

I am ready to make PR when details are discussed and approved.

[tchaton]

Dear @sharuevds,

Sounds like a great idea !
Would you mind making a PR for it ?

Best,
T.C

[borisdayma]

The difficulty here will be that integrations may allow different options.

For example with wandb:

• segmentation masks accept labels
• bounding boxes can be specified with different options
• audio can accept caption and sample rate
• video accepts fps and format
• very specific formats are also accepted: html, molecules…

I really love the idea though! We should just try to provide as many options as possible to take best advantage of loggers capabilities.

[stale]

This issue has been automatically marked as stale because it hasn't had any recent activity. This issue will be closed in 7 days if no further activity occurs. Thank you for your contributions, Pytorch Lightning Team!

[Queuecumber]

Would love to see this as well, there should be a way to abstract the API enough to cover all use cases. Worst case scenario these functions can accept kwargs or something if you know what logger you're interacting with. I can try to mock something up if my currently pending PR ever gets merged

[ibro45]

I would suggest to make these a part of LightningModule, just like log() or log_dict(). It would be a pity to have to call them from self.logger.experiment when they are already standardized. Example:

self.log("loss", loss, on_step=True, on_epoch=True)
self.log_dict("metrics", metrics, on_step=True, on_epoch=True)

self.logger.experiment.log_images("input", input)  # pls no
# vs
self.log_images("input", input)

Making them part of the LightningModule would also be great because we could have on_step and on_epoch for them!

self.log_images("input", input, on_step=True, on_epoch=True)

Users could still access the logger-specific methods from self.logger.experiments, but that would be for advanced use-cases.

What do you think?

[borisdayma]

For info it has already been implemented as part of master branch (see here)
You have access to log_image and log_text directly from the LightningModule and args are passed to the specific loggers.
I guess this issue can be closed?

[ibro45]

Actually, it is not implemented, I just tried with master branch:

• LightningModule doesn't have log_image nor log_text, so calling self.log_image() raises AttributeError.
• Calling self.logger.log_image() raises NotImplementedError when using TensorboardLogger.
• In addition to TensorboardLogger, I see that log_image and log_text aren't implemented for MLFlowLogger and CometLogger either.
• log_image and log_text are implemented for WandbLogger and NeptuneLogger, but they have different names for the same kind of keyword argument (e.g. key and log_names), which is problematic if you try to use both.

[twsl]

I think the first simple step is to replace the exceptions in the LightningLoggerBase with pass. I'll try to see if I can find some time.

[carmocca]

As described in #12183 (comment) and the rest of the comments, closing.