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

ng: reload now only reloads current dashboard #3426

Merged
merged 3 commits into from
Mar 25, 2020

Conversation

stephanwlee
Copy link
Contributor

Previously, we were reloading all stamped dashboards which is not the
behavior of the tf-tensorboard.

Side-effect: because we were reloading scalars dashboard when it was in
the background, the charts could more frequently appear tiny when the
reload kicked in. This change removes that bug.

Previously, we were reloading all stamped dashboards which is not the
behavior of the tf-tensorboard.

Side-effect: because we were reloading scalars dashboard when it was in
the background, the charts could more frequently appear tiny when the
reload kicked in. This change removes that bug.
Copy link
Contributor

@psybuzz psybuzz left a comment

Choose a reason for hiding this comment

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

Looks good!

  1. Can the original bug (small charts) still technically occur, if an inactive dashboard's reload() is still pending?

  2. This behavior now matches the Polymer tf-tensorboard, but I'd like to question the ideal UX. Feel free to defer/not answer this part, if it is beyond the scope.

Let's say a user opens Scalars at t = 0, opens Images, and autoreloads until t = 5000s, then returns to Scalars. For up to 30s, they'll experience 2 dashboards with 5000s difference in data freshness.

Would it make sense to mark dashboards as "dirty" when they are inactive during an autoreload, and explicitly reload() upon re-opening them? That way, the data freshness gap between any 2 dashboards should be about < 1 reload period.

tensorboard/webapp/plugins/plugins_container_test.ts Outdated Show resolved Hide resolved
@stephanwlee
Copy link
Contributor Author

  1. Can the original bug (small charts) still technically occur, if an inactive dashboard's reload() is still pending?

Correct

  1. This behavior now matches the Polymer tf-tensorboard, but I'd like to question the ideal UX. Feel free to defer/not answer this part, if it is beyond the scope.
    Let's say a user opens Scalars at t = 0, opens Images, and autoreloads until t = 5000s, then returns to Scalars. For up to 30s, they'll experience 2 dashboards with 5000s difference in data freshness.

Beyond scope and the same behavior is what I am trying to achieve here. Your assessment is correct and it is out of the scope.

@stephanwlee stephanwlee merged commit 69953d1 into tensorflow:master Mar 25, 2020
@stephanwlee stephanwlee deleted the reload branch March 25, 2020 01:54
bileschi pushed a commit to bileschi/tensorboard that referenced this pull request Apr 15, 2020
Previously, we were reloading all stamped dashboards which is not the
behavior of the tf-tensorboard.

Side-effect: because we were reloading scalars dashboard when it was in
the background, the charts could more frequently appear tiny when the
reload kicked in. This change removes that bug.
bileschi pushed a commit that referenced this pull request Apr 15, 2020
Previously, we were reloading all stamped dashboards which is not the
behavior of the tf-tensorboard.

Side-effect: because we were reloading scalars dashboard when it was in
the background, the charts could more frequently appear tiny when the
reload kicked in. This change removes that bug.
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.

3 participants