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.

Sign up for GitHub

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

Jump to bottom

Issues with Release 2.15.0 #6651

Closed
ibaris opened this issue Oct 20, 2023 · 2 comments
Closed

Issues with Release 2.15.0 #6651

ibaris opened this issue Oct 20, 2023 · 2 comments
Assignees
@JamesHollyer @rileyajones
Labels
core:frontend plugin:time-series

Comments

@ibaris
Copy link

ibaris commented Oct 20, 2023 •
edited

I'd like to provide some feedback on the latest release of the software. While I appreciate the efforts put into it, there are some aspects that make it feel more like a beta release rather than a stable one. Here are a few observations:

  1. Visual Chunkiness: The user interface seems to have become bulkier compared to previous versions, which affects the overall user experience.

  2. Dark Mode Issues: In the dark mode design, there are readability issues with text on buttons, particularly the "Next" and "Previous" buttons.

  3. Data Table Persistence: I've noticed that I cannot disable the new data table at the bottom of the plots, which can be inconvenient. Edit: I found out, that the Enable step selection and data table was selected by default.

  4. Matplotlib Image Display: Images generated with Matplotlib do not appear correctly within the application.

  5. TensorBoard Behavior: When TensorBoard refreshes, the left column table containing all the experiments jumps to the top, which can be problematic when working with a large number of experiments.

  6. Sorting: The sorting of the experiments seems to be wrong. I have 150 experiments and the sorting is like: 1, 100, 101, ...

I hope these observations can help improve the software's usability and stability in future releases. Thank you for your hard work!
@JamesHollyer
Copy link
Contributor

JamesHollyer commented Oct 20, 2023 •
edited

Thank you so much for your feedback. Many of these changes are due to an Angular mandate to move from the "legacy" material components.

  1. This is the new look for Angular Material Components. I do not think it looks that bad. If you have any specific examples we may be able to look into it.
  2. Sorry we missed those! I will address this ASAP.
  3. Yes, we turned that on by default. I hope it is not too inconvenient to disable. Your disabled selection will persist. You can also turn off the table for individual charts by clicking the little "x" in the step selector fob.
  4. This is confusing. I am not sure why those images would be affected. Could you share a screenshot of what is happening?
  5. I didn't even know we had that behavior previously. I will look into this.
  6. Yea that is a problem too. We will address.

As I said many of these issues are from the mandated Angular Material Components update. However, some are due to the new Runs Table which has a lot more functionality as well as the ability to add much more functionality in future versions. Currently the only way to access that new functionality is to log hparams(here is a demo showing how that is done https://github.com/tensorflow/tensorboard/blob/master/tensorboard/plugins/hparams/hparams_demo.py). In future versions these features will be even more accessible. I hope you find them useful!

If you find any other problems we missed please add them here and I will take a look

This was referenced Oct 20, 2023
Closed
Merged
JamesHollyer added a commit that referenced this issue Oct 23, 2023
@JamesHollyer
HParams: Load spinner in data table and do not reload table (#6658)
7f7d140 
## Motivation for features / changes
Issue number 5 in #6651. It is also just a better experience to keep
previously loaded data intact while fetching new data.

## Technical description of changes
In order to put the spinner below the headers I added the spinner to the
data-table. It seems like a general feature that should be in that
widget anyway.

## Screenshots of UI changes (or N/A)
![2023-10-20_15-13-34
(1)](https://github.com/tensorflow/tensorboard/assets/8672809/23bfa0ed-077c-45fb-9df5-a0c3db1db67d)
@JamesHollyer JamesHollyer mentioned this issue Oct 23, 2023
Merged
JamesHollyer added a commit that referenced this issue Oct 23, 2023
@JamesHollyer
MDCMigration: Fix prev and next buttons for dark mode (#6663)
8ebecb7 
## Motivation for features / changes
The new mat-button uses different colors in it's styling. This caused a
readability problem with our Next and Prev buttons when enabled. This
fixes the buttons to be readable and look nice. We decided the
transparent background looked nice in dark mode but not in light mode.

This was brought to our attention in #6651.

## Screenshots of UI changes (or N/A)
Before:
<img width="905" alt="Screenshot 2023-10-20 at 11 22 09 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/ffad969c-4fa0-4f45-af80-0975a5ad2756">
<img width="897" alt="Screenshot 2023-10-20 at 11 22 17 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/5fc01f59-7e6c-407d-929b-aac6bdcf0ac6">

After:
<img width="868" alt="Screenshot 2023-10-20 at 11 00 38 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/e8000574-910e-4fef-bb44-26b4faeb5966">
<img width="887" alt="Screenshot 2023-10-20 at 11 13 09 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/18b821d0-1942-43cd-a501-5d5b7549a8a6">

## Alternate designs / implementations considered (or N/A)
Also considered having a transparent background in light mode.
@rileyajones rileyajones mentioned this issue Oct 24, 2023
Merged
rileyajones added a commit that referenced this issue Oct 25, 2023
@rileyajones
Sort run names with leading numbers differently (#6664)
e8dd1ca 
## Motivation for features / changes
We have been treating all run names as strings even though some run
names are (serialized) numbers and some begin with numbers. This means
that sorting worked pretty unintuitively.

Note: I've also changed the behavior around sorting `undefined` values.
When sorting descending they should now appear at the top of the list.
This was a recent change but it wasn't clear it was intentional and I
found it made the code more complex.

#6651

Internal users see [b/278671226](http://b/278671226)

## Screenshots of UI changes (or N/A)

Before:

![image](https://github.com/tensorflow/tensorboard/assets/78179109/5f805c37-283d-4f55-b1bf-3dfa4d9ea1da)

After:

![image](https://github.com/tensorflow/tensorboard/assets/78179109/0d740b6e-2ed5-4762-aec0-22400eeb152d)


![image](https://github.com/tensorflow/tensorboard/assets/78179109/5283bade-b4da-401d-9893-932d9fb9d378)
@JamesHollyer
Copy link
Contributor

I think all of the addressable issues have been dealt with. The only remaining issue the matplotlib image display. I have asked for some more details here. If you can provide a screenshot or some more details please do so and reopen this issue.

Thanks again for the feedback!

bmd3k pushed a commit to bmd3k/tensorboard that referenced this issue Nov 2, 2023
@JamesHollyer @bmd3k
HParams: Load spinner in data table and do not reload table (tensorfl…
27409c5 
…ow#6658)

## Motivation for features / changes
Issue number 5 in tensorflow#6651. It is also just a better experience to keep
previously loaded data intact while fetching new data.

## Technical description of changes
In order to put the spinner below the headers I added the spinner to the
data-table. It seems like a general feature that should be in that
widget anyway.

## Screenshots of UI changes (or N/A)
![2023-10-20_15-13-34
(1)](https://github.com/tensorflow/tensorboard/assets/8672809/23bfa0ed-077c-45fb-9df5-a0c3db1db67d)
bmd3k pushed a commit to bmd3k/tensorboard that referenced this issue Nov 2, 2023
@JamesHollyer @bmd3k
MDCMigration: Fix prev and next buttons for dark mode (tensorflow#6663)
e6355e8 
## Motivation for features / changes
The new mat-button uses different colors in it's styling. This caused a
readability problem with our Next and Prev buttons when enabled. This
fixes the buttons to be readable and look nice. We decided the
transparent background looked nice in dark mode but not in light mode.

This was brought to our attention in tensorflow#6651.

## Screenshots of UI changes (or N/A)
Before:
<img width="905" alt="Screenshot 2023-10-20 at 11 22 09 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/ffad969c-4fa0-4f45-af80-0975a5ad2756">
<img width="897" alt="Screenshot 2023-10-20 at 11 22 17 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/5fc01f59-7e6c-407d-929b-aac6bdcf0ac6">

After:
<img width="868" alt="Screenshot 2023-10-20 at 11 00 38 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/e8000574-910e-4fef-bb44-26b4faeb5966">
<img width="887" alt="Screenshot 2023-10-20 at 11 13 09 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/18b821d0-1942-43cd-a501-5d5b7549a8a6">

## Alternate designs / implementations considered (or N/A)
Also considered having a transparent background in light mode.
bmd3k pushed a commit to bmd3k/tensorboard that referenced this issue Nov 2, 2023
@rileyajones @bmd3k
Sort run names with leading numbers differently (tensorflow#6664)
9b76dbc 
## Motivation for features / changes
We have been treating all run names as strings even though some run
names are (serialized) numbers and some begin with numbers. This means
that sorting worked pretty unintuitively.

Note: I've also changed the behavior around sorting `undefined` values.
When sorting descending they should now appear at the top of the list.
This was a recent change but it wasn't clear it was intentional and I
found it made the code more complex.

tensorflow#6651

Internal users see [b/278671226](http://b/278671226)

## Screenshots of UI changes (or N/A)

Before:

![image](https://github.com/tensorflow/tensorboard/assets/78179109/5f805c37-283d-4f55-b1bf-3dfa4d9ea1da)

After:

![image](https://github.com/tensorflow/tensorboard/assets/78179109/0d740b6e-2ed5-4762-aec0-22400eeb152d)


![image](https://github.com/tensorflow/tensorboard/assets/78179109/5283bade-b4da-401d-9893-932d9fb9d378)
bmd3k pushed a commit that referenced this issue Nov 2, 2023
@JamesHollyer @bmd3k
HParams: Load spinner in data table and do not reload table (#6658)
6fa9fa9 
## Motivation for features / changes
Issue number 5 in #6651. It is also just a better experience to keep
previously loaded data intact while fetching new data.

## Technical description of changes
In order to put the spinner below the headers I added the spinner to the
data-table. It seems like a general feature that should be in that
widget anyway.

## Screenshots of UI changes (or N/A)
![2023-10-20_15-13-34
(1)](https://github.com/tensorflow/tensorboard/assets/8672809/23bfa0ed-077c-45fb-9df5-a0c3db1db67d)
bmd3k pushed a commit that referenced this issue Nov 2, 2023
@JamesHollyer @bmd3k
MDCMigration: Fix prev and next buttons for dark mode (#6663)
6c0319e 
## Motivation for features / changes
The new mat-button uses different colors in it's styling. This caused a
readability problem with our Next and Prev buttons when enabled. This
fixes the buttons to be readable and look nice. We decided the
transparent background looked nice in dark mode but not in light mode.

This was brought to our attention in #6651.

## Screenshots of UI changes (or N/A)
Before:
<img width="905" alt="Screenshot 2023-10-20 at 11 22 09 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/ffad969c-4fa0-4f45-af80-0975a5ad2756">
<img width="897" alt="Screenshot 2023-10-20 at 11 22 17 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/5fc01f59-7e6c-407d-929b-aac6bdcf0ac6">

After:
<img width="868" alt="Screenshot 2023-10-20 at 11 00 38 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/e8000574-910e-4fef-bb44-26b4faeb5966">
<img width="887" alt="Screenshot 2023-10-20 at 11 13 09 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/18b821d0-1942-43cd-a501-5d5b7549a8a6">

## Alternate designs / implementations considered (or N/A)
Also considered having a transparent background in light mode.
bmd3k pushed a commit that referenced this issue Nov 2, 2023
@rileyajones @bmd3k
Sort run names with leading numbers differently (#6664)
f28d5c0 
## Motivation for features / changes
We have been treating all run names as strings even though some run
names are (serialized) numbers and some begin with numbers. This means
that sorting worked pretty unintuitively.

Note: I've also changed the behavior around sorting `undefined` values.
When sorting descending they should now appear at the top of the list.
This was a recent change but it wasn't clear it was intentional and I
found it made the code more complex.

#6651

Internal users see [b/278671226](http://b/278671226)

## Screenshots of UI changes (or N/A)

Before:

![image](https://github.com/tensorflow/tensorboard/assets/78179109/5f805c37-283d-4f55-b1bf-3dfa4d9ea1da)

After:

![image](https://github.com/tensorflow/tensorboard/assets/78179109/0d740b6e-2ed5-4762-aec0-22400eeb152d)


![image](https://github.com/tensorflow/tensorboard/assets/78179109/5283bade-b4da-401d-9893-932d9fb9d378)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JamesHollyer JamesHollyer

@rileyajones rileyajones

Labels
core:frontend plugin:time-series
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@JamesHollyer @ibaris @bmd3k @rileyajones