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

docs: document the Time Series dashboard #5193

Merged
merged 2 commits into from
Aug 5, 2021
Merged

Conversation

psybuzz
Copy link
Contributor

@psybuzz psybuzz commented Aug 4, 2021

This adds documentation that is both user-facing and contributor-facing,
for the Time Series dashboard. This does not touch any of the guides or
tutorials hosted at tensorflow.org.

See #4660

@google-cla google-cla bot added the cla: yes label Aug 4, 2021
@psybuzz psybuzz marked this pull request as ready for review August 4, 2021 22:29
@psybuzz psybuzz requested a review from nfelt August 4, 2021 22:29
Copy link
Contributor

@nfelt nfelt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for getting to this!

Notably, the frontend is aware of a few concepts

* Experiment: the currently selected logdir of a TensorBoard instance represents a fixed scope of runs. In this sense, an 'experiment' represents the collection of all the runs in the logdir.
* RunId: users currently provided a run name during summary writing, for example 'run_directory_1' in `tf.summary.create_file_writer('run_directory_1')`. To support a future with multiple experiments, the RunId concept is a unique identifier of a single run in a specific experiment. Multiple experiments can have a run with the same name 'run_directory_1', but each run will have its own RunId.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/provided/provide/ ? "currently" + past tense reads weird IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


* Autocomplete in tag filter: search for specific charts more easily.

## Implementation notes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content here feels like it's a little more on the side of a design doc or a README for plugin developers - it's not really something users would need to know. That seems a little at odds with the location of this doc, since currently this page (docs/timeseries_dashboard.md) is alongside our user-facing plugin docs that are actually published on tensorflow.org/tensorboard. (This one would also end up published there, although since it's not in the YAML file it wouldn't be very discoverable.)

Conventionally, the more developer-oriented docs have gone in plugins/foo/README.md - sometimes that content is also more user-facing but if there's developer-facing content, that's where we put it, e.g.

So I'd recommend putting this content there, and then either omitting this file, or including only the user-facing aspects there (and in that case also adding it to the YAML file so it's linked from the site nav).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omitted this file for now, and moved contents to plugins/metrics/README.md.

Makes sense. I think omitting the user-facing one in this PR would be better, so a separate PR could address it and add to the YAML later.

README.md Outdated
@@ -235,6 +235,23 @@ projector tutorial](https://www.tensorflow.org/tutorials/text/word_embeddings).
The Text Dashboard displays text snippets saved via `tf.summary.text`. Markdown
features including hyperlinks, lists, and tables are all supported.

### Time Series Dashboard (new)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might omit (new) - it is new-er yes, but not that new (released in 2.4.0 last November), and this README is already so stale that I foresee this still saying (new) 10 years from now if left in ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@psybuzz psybuzz merged commit e48001c into tensorflow:master Aug 5, 2021
yatbear pushed a commit to yatbear/tensorboard that referenced this pull request Mar 27, 2023
This adds documentation that is both user-facing and contributor-facing,
for the Time Series dashboard. This does not touch any of the guides or
tutorials hosted at tensorflow.org.

See tensorflow#4660
dna2github pushed a commit to dna2fork/tensorboard that referenced this pull request May 1, 2023
This adds documentation that is both user-facing and contributor-facing,
for the Time Series dashboard. This does not touch any of the guides or
tutorials hosted at tensorflow.org.

See tensorflow#4660
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants