HParams: Start showing hparams in runs table based on the enableHparamsInTimeSeries flag #6317
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rileyajones
added a commit
that referenced
this pull request
Apr 14, 2023
## Motivation for features / changes As part of the effort to surface hparams data in the time series dashboard we need to start actually fetching it. The logic existed internally but for some reason was not previously available in OSS. ## Technical description of changes I just copied the internal code to the oss implementation and made a few small adjustments to imports ## Screenshots of UI changes None ## Detailed steps to verify changes work correctly (as executed by you) Tests should pass Patch #6317, enable the feature flag, and ensure the hparams appear in the runs table ## Alternate designs / implementations considered Make a more detailed stub?
rileyajones
force-pushed
the
hparams-filter
branch
from
23e6d05 to
66d468a
Compare
JamesHollyer
approved these changes
Apr 14, 2023
| }); | ||
|
|
||
| it('renders', () => { | ||
| store.overrideSelector(getEnableHparamsInTimeSeries, false); |
Contributor
There was a problem hiding this comment.
Not a big deal since these are the only 2 tests but, this should probably be in the beforeEach.
Contributor
Author
There was a problem hiding this comment.
Since this is not always the same I would actually prefer to keep it overridden here for now.
rileyajones
added a commit
that referenced
this pull request
Apr 17, 2023
## Motivation for features / changes #6317 broke the internal sync Googlers see (cl/524838273) This should fix it Googlers see cl/524852584
dna2github
pushed a commit
to dna2fork/tensorboard
that referenced
this pull request
May 1, 2023
…w#6318) ## Motivation for features / changes As part of the effort to surface hparams data in the time series dashboard we need to start actually fetching it. The logic existed internally but for some reason was not previously available in OSS. ## Technical description of changes I just copied the internal code to the oss implementation and made a few small adjustments to imports ## Screenshots of UI changes None ## Detailed steps to verify changes work correctly (as executed by you) Tests should pass Patch tensorflow#6317, enable the feature flag, and ensure the hparams appear in the runs table ## Alternate designs / implementations considered Make a more detailed stub?
dna2github
pushed a commit
to dna2fork/tensorboard
that referenced
this pull request
May 1, 2023
…msInTimeSeries flag (tensorflow#6317) Nothing will be displayed until ~tensorflow#6318~ is merged ## Motivation for features / changes We are working on surfacing hparam data in the time series dashboard. As part of this effort we would like to show the value of hparams in the runs table. The runs table already has partial support for this but it is currently disabled. I am enabling the columns based on the feature flag I added in tensorflow#6308. Note that you will not see anything in OSS until we fix the call to session_groups ## Technical description of changes I added a new selector to get the feature flag, then modified the metrics_container to read the value of the flag ## Screenshots of UI changes Disabled  Enabled  ## Detailed steps to verify changes work correctly (as executed by you) ## Alternate designs / implementations considered
dna2github
pushed a commit
to dna2fork/tensorboard
that referenced
this pull request
May 1, 2023
## Motivation for features / changes tensorflow#6317 broke the internal sync Googlers see (cl/524838273) This should fix it Googlers see cl/524852584
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Nothing will be displayed until
#6318is mergedMotivation for features / changes
We are working on surfacing hparam data in the time series dashboard. As part of this effort we would like to show the value of hparams in the runs table. The runs table already has partial support for this but it is currently disabled.
I am enabling the columns based on the feature flag I added in #6308.
Note that you will not see anything in OSS until we fix the call to session_groups
Technical description of changes
I added a new selector to get the feature flag, then modified the metrics_container to read the value of the flag
Screenshots of UI changes
Disabled
Enabled
Detailed steps to verify changes work correctly (as executed by you)
Alternate designs / implementations considered