-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Log TensorBoard
histograms
#19743
Labels
Comments
dominicgkerr
added
feature
Is an improvement or enhancement
needs triage
Waiting to be triaged by maintainers
labels
Apr 7, 2024
@Borda Hello! Do you have any thoughts or concerns about this feature request? Would opening a PR implementing the above be helpful? Let me know if you need anymore information. Many thanks |
10 tasks
Hi, I've open a PR (#19851) to help illustrate the above description/request. Please let me know if there's anything else I can do to help - thanks |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Description & Motivation
I'd like to be able to log histograms using
TensorBoard
. Looking at how multiple/single scalars are currently logged bylightning.fabric.loggers.TensorBoardLogger
[1], I thinkSummaryWriter.add_histogram
[2] could be called (non-emptytorch.Tensor
instances) in a similar wayUnfortunately, values passed into
LightningModule.log
are first type-checked byLightningModule.__to_tensor
[3] - this enforces values to be eitherdict
or single-value (0Dim)torch.Tensor
instances. So in order to support histograms, this check would need to be loosen slightly...Pitch
Firstly, I'm proposing [4] becomes:
and [1] becomes (say):
I'm sure you're keen not to modify
LightnginModule
too much, but I couldn't figure out a way to pass 1+DimTensor
instances through to theTensorBoardLogger
without loosing the type checking.Alternatives
As a workaround, I'm currently using a subclass of
LightningModule
that overloads the.log
method [5] with:Introducing
TensorBoard
specifics into theLightningModule
class, is a horrible hack (hence wanting to incorporate the "nicer" fix above!)Additional context
I'm very happy to open a PR (I have one written up from exploring potential implementations) is the above sounds reasonable?
Many thanks!
cc @Borda
The text was updated successfully, but these errors were encountered: