Skip to content
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

Feature request: Add training information to tensorboard scalars #643

Closed
phemmer opened this issue Jan 5, 2020 · 5 comments
Closed

Feature request: Add training information to tensorboard scalars #643

phemmer opened this issue Jan 5, 2020 · 5 comments
Labels
enhancement New feature or request RTFM Answer is the documentation

Comments

@phemmer
Copy link

phemmer commented Jan 5, 2020

I think it would be a good change to add the training information metrics that are output to the logger (when verbose>=1) to tensorboard as well.

I've occasionally run into issues where looking at a snapshot of the values (the log output to the console) hasn't been obviously problematic, but looking at the values over time has made an issue apparent.
Currently I'm copying the code that generates the logger values out of the model and into my own custom callback where I write to tensorboard. But I would like to leave that code in the model and not have to duplicate it.

Making the change is likely trivial, as it would just involve taking the values from loger.getkvs(). But I wanted to solicit feedback before creating a PR, as there's a few ways to go about it, and don't want to waste the time if there's no chance it'd get merged.

My thought on implementation would be to make logger.dumpkvs() accept an optional parameter for a TensorboardWriter, and when present, would dump the values there in addition to the console.
However this does mean that the tensorboard output would be linked to setting verbose>=1. If someone wants to output to tensorboard, but not console, it wouldn't be possible. An alternative could be a separate method on logger specifically for dumping to tensorboard, but not sure how the model parameters should look to toggle that behavior.

@araffin araffin added enhancement New feature or request RTFM Answer is the documentation labels Jan 5, 2020
@araffin
Copy link
Collaborator

araffin commented Jan 5, 2020

Hello,

have you read the documentation ?

@phemmer
Copy link
Author

phemmer commented Jan 5, 2020

Yes, I called that out in the issue description:

Currently I'm copying the code that generates the logger values out of the model and into my own custom callback where I write to tensorboard. But I would like to leave that code in the model and not have to duplicate it.

P.S. Before throwing an RTFM tag on the issue, I might suggest RTFI

@araffin
Copy link
Collaborator

araffin commented Jan 5, 2020

Currently I'm copying the code that generates the logger values

Why are you doing that where you can enable logging the values with tensorboard with

# formats are comma-separated, but for tensorboard you only need the last one
# stdout -> terminal
export OPENAI_LOG_FORMAT='stdout,log,csv,tensorboard'
export OPENAI_LOGDIR=path/to/tensorboard/data

which avoid using callbacks?

@phemmer
Copy link
Author

phemmer commented Jan 5, 2020

I apologize then, as I misunderstood that section of the documentation, and thought it was saying something else. That solution isn't great (it creates a seaparate run in tensorboard instead of being part of the same run, and it creates a ton of top-level tags, instead of putting them all under logger, which would be cleaner IMHO). But as the functionality obviously exists, it does not need to be added, and I'll close the issue.

@phemmer phemmer closed this as completed Jan 5, 2020
@araffin
Copy link
Collaborator

araffin commented Jan 5, 2020

That solution isn't great

yes, I know, that's why it is the legacy integration, I agree on the fact it could be better (hence the enhancement tag). Another thing to add to the list of v3 ( #576 and #366 ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request RTFM Answer is the documentation
Projects
None yet
Development

No branches or pull requests

2 participants