-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
TensorBoard/Wandb/optuna/raytune integration improvements. #7935
Conversation
…tune support, with minor modifications to trainer core code.
3a7a0ee
to
5191210
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think changing the metrics names in the logs is a good idea as there are other components relying on them. Renaming the keys inside the TensorBoardCallback to make tensorboard happier seems fine but outside of it, I would prefer to avoid any breaking change.
e339f01
to
55fa620
Compare
55fa620
to
d57d45f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing/making all of this better!
…ce#7935) Improved TensorBoard and Wandb integration, as well as optuna and ray/tune support, with minor modifications to trainer core code.
…uggingface#7935)" This reverts commit e07eb0d.
Improves TensorBoard logging by grouping train / eval metrics as it is usually done in TensorBoard.
Improves TensorBoard/optuna model hyper-parameters logging.
Improves optuna and Ray/tune integration, and provides model hyper-parameter naming.
Test (and sample code) is provided in test_trainer.TrainerHyperParameterIntegrationTest .
Some more work may be need to harmonize metrics naming for eval / train, as the "eval_" prefix used is not very convenient, using a "eval/" prefix would be more foolproof, and consistent with TensorBoard usage, but it would break quite some code, and so may be done in a separate PR.