Skip to content

Conversation

@rileyajones
Copy link
Contributor

Motivation for features / changes

As part of the effort to bring hparams to the time series dashboard a handful of new selectors need to be created. In particular I will be creating a new one to handle runs filtering by moving the logic out of the runs_table_container into the selector layer.
This will ensure filtered runs are consistent across the dashboard.

Technical description of changes

This new selector is not meaningfully different from the existing one but the factory allows it to be called with an encapsulated experiment id.

@rileyajones rileyajones requested a review from JamesHollyer May 2, 2023 23:48
@rileyajones rileyajones marked this pull request as ready for review May 3, 2023 00:29
...state.runMetadata[runId],
experimentId,
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer something like this:

(state.runsIds[experimentId] || []).forEach(runId => {
  if(state.runMetadata[id]) {
    runs.push({...state.runMetadata[runId], exprimentId});
  }
})

inside the reduce function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the formatting to meet someplace in between

@rileyajones rileyajones requested a review from JamesHollyer May 3, 2023 22:10
@rileyajones rileyajones merged commit 49cc84a into tensorflow:master May 3, 2023
rileyajones added a commit to rileyajones/tensorboard that referenced this pull request May 5, 2023
…nsorflow#6360)

## Motivation for features / changes
As part of the effort to bring hparams to the time series dashboard a
handful of new selectors need to be created. In particular I will be
creating a new one to handle runs filtering by moving the logic out of
the runs_table_container into the selector layer.
This will ensure filtered runs are consistent across the dashboard.

## Technical description of changes
This new selector is not meaningfully different from the existing one
but the factory allows it to be called with an encapsulated experiment
id.
rileyajones added a commit that referenced this pull request May 9, 2023
## Motivation for features / changes
The runs table currently contains the logic to filter runs by hparams,
unfortunately, this logic is not shared with the rest of the dashboard
and so filtering runs this way does not actually effect what is shown on
the dashboard.

## Technical description of changes
I moved out a lot of utility functions from the runs table container
into common selectors. I then created three selectors
1) `getRenderableRuns`
A private selector to help with the readability of
`getFilteredRenderableRuns`
  Gets all runs associated with the provided experiment ids
2) `getFilteredRenderableRuns`
Gets all runs associated with the provided experiment ids while
accounting for filters.
3) `getFilteredRenderableRunsFromRoute`
Retrieves the experiment ids from the route then gets the filtered runs
associated with them

NOTE: The utility functions are encapsulated in a utils object so that
they can be spied on during testing

See the PRs leading up to this
#6340 #6360 #6361 #6323 #6324

## Screenshots of UI changes (or N/A)
N/A
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.

2 participants