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: Allow users to load more Hparams if they are not all included in default. #6780

Merged
merged 2 commits into from
Mar 11, 2024

Conversation

bmd3k
Copy link
Contributor

@bmd3k bmd3k commented Mar 7, 2024

In #6777 we limited the number of hparams we load by default.

Now we add the option for the user to load all hparams in spite of the performance implications.

OSS Light Mode:
image

OSS Dark Mode:
image

Internal Light Mode:
image

Internal Dark Mode:
image

@bmd3k bmd3k force-pushed the hparams-20240307-load-more-anyways-2 branch from 26e5e85 to 170284d Compare March 7, 2024 21:56
@bmd3k bmd3k force-pushed the hparams-20240307-load-more-anyways-2 branch from 170284d to f968643 Compare March 8, 2024 11:46
@bmd3k bmd3k requested a review from rileyajones March 8, 2024 12:32
Instead of the way I was previously removing the flakiness.
<button mat-stroked-button (click)="loadAllColumnsClicked()">
Load all anyway
</button>
</div>
</div>

<div #columnList class="column-list">
Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to wrap the list of columns in a virtual scroll viewbox prior to launching the data project
https://material.angular.io/cdk/scrolling/overview#cdk-virtual-scroll-overview

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, added that suggestion to our list of tasks. We should at least play with it.

@bmd3k bmd3k merged commit da812d2 into tensorflow:master Mar 11, 2024
13 checks passed
bmd3k added a commit that referenced this pull request Mar 22, 2024
We limit the number of hparams retrieved and rendered in the Hparams
plugin. Otherwise the plugin can become very slow and even crash.

Unlike #6780, we do not provide an option for loading more in the
Hparams plugin. We may provide this option later if there is demand. But
generally we think 1000 should be enough, especially if the underlying
hparams provider is sorting the results with "differs" fields first.


![image](https://github.com/tensorflow/tensorboard/assets/17152369/da1f2c26-3804-4faf-be4a-03bb8aaa178d)
AnuarTB pushed a commit to AnuarTB/tensorboard that referenced this pull request May 3, 2024
AnuarTB pushed a commit to AnuarTB/tensorboard that referenced this pull request May 3, 2024
…low#6807)

We limit the number of hparams retrieved and rendered in the Hparams
plugin. Otherwise the plugin can become very slow and even crash.

Unlike tensorflow#6780, we do not provide an option for loading more in the
Hparams plugin. We may provide this option later if there is demand. But
generally we think 1000 should be enough, especially if the underlying
hparams provider is sorting the results with "differs" fields first.


![image](https://github.com/tensorflow/tensorboard/assets/17152369/da1f2c26-3804-4faf-be4a-03bb8aaa178d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants