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

hparams: fix the parallel coordinate layout #4988

Merged
merged 1 commit into from
May 19, 2021

Conversation

stephanwlee
Copy link
Contributor

@stephanwlee stephanwlee commented May 19, 2021

Bug

Previously, we were setting the width of the container based on count of
all hparams and metrics without considering the visibility. This had
caused the UI to be in very awkward state where each visible hparams and
metrics are spaced a lot even if the selection count is quite low.

Fix

We layout the parallel coordinates based on number of visible
{hparam,metric}s. The change is a bit larger because:

  • I had to add type information and refactor it a little for
    understanding the code (minimally added the types).
  • Undo the performance optimization where we do not mutate the layout
    unless important piece of information in the configuration has
    changed. To detect the old configuration vs. new configuration, we
    have to now remember an additional state in the component.

Fixes #3414.

Previous:
image

New:
image

Bug
---
Previously, we were setting the width of the container based on count of
all hparams and metrics without considering the visibility. This had
caused the UI to be in very awkward state where each visible hparams and
metrics are spaced a lot even if the selection count is quite low.

Fix
---
We layout the parallel coordinates based on number of visible
{hparam,metric}s. The change is a bit larger because:

- I had to add type information and refactor it a little for
  understanding the code.
- Undo the performance optimization where we do not mutate the layout
  unless important piece of information in the configuration has
  changed. To detect the old confiugration vs. new configuration, we
  have to now remember an additional state in the component.

Fixes tensorflow#3414.
@stephanwlee stephanwlee requested a review from psybuzz May 19, 2021 18:59
@google-cla google-cla bot added the cla: yes label May 19, 2021
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.

Thanks for adding type interfaces too!

@stephanwlee stephanwlee merged commit 920dd1f into tensorflow:master May 19, 2021
@stephanwlee stephanwlee deleted the hparams branch May 19, 2021 21:51
@mhamilton723
Copy link

@stephanwlee @psybuzz is this released yet im having these issues with 2.5.0. Thanks!

@psybuzz
Copy link
Contributor

psybuzz commented Aug 4, 2021

Thanks for the ping. The fix will be part of the upcoming TensorBoard 2.6.0 (and TensorFlow 2.6.0) release, so please stay tuned. The TB cadence has roughly matched the TensorFlow release cadence, so the ETA is soon (on the order of days/weeks from now, not months).

If you're in need of this fix immediately, it should be available in https://pypi.org/project/tb-nightly/ right now.

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.

hparams parallel coordinate view should adjust width for number of axes
3 participants