-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[WIP] tensorboard-notf for pytorch (re-submit of #1799) #1801
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
base: master
Are you sure you want to change the base?
Conversation
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.
@nfelt, we're getting closer on the split of tensorboardX into tensorflow/tensorboard and pytorch/pytorch. Would you mind doing a review here and give us some pointers on where the code should live? Much appreciated.
@@ -0,0 +1,25 @@ | |||
/* Copyright 2017 The TensorFlow Authors. All Rights Reserved. |
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.
Can we use the version of this in the plugin directory instead?
https://github.com/tensorflow/tensorboard/blob/master/tensorboard/plugins/pr_curve/plugin_data.proto
@@ -0,0 +1,96 @@ | |||
/* Copyright 2017 The TensorFlow Authors. All Rights Reserved. |
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.
Can we use the version in the plugin directory?
https://github.com/tensorflow/tensorboard/blob/master/tensorboard/plugins/custom_scalar/layout.proto
@@ -0,0 +1,27 @@ | |||
/* Copyright 2017 The TensorFlow Authors. All Rights Reserved. |
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.
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.
replaces plugin_text.proto and plugin_pr_curve.proto with plugin_data.proto will lead to name confliction. How about using soft links?
@@ -0,0 +1,22 @@ | |||
# Description: |
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.
Instead of compat/tensorboard
can we do compat/writer
? @nfelt what are your thoughts on the right naming for this?
"types.proto", | ||
"verifier_config.proto", | ||
"versions.proto", | ||
"plugin_text.proto", # relative dir is not accepted. copt that. |
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 think we can actually get all three of these from the plugins/ directory.
@lanpa, I just saw that you have direct reference of the protos on your TODO list. Excellent! |
This modification let pytorch/pytorch@00b8330 use the code inside tensorboard installed by
whl
file build by bazel.The generated event file can be rendered correctly by web server build from:
#1663
TODO: