From 62f671a24eb451e380ef11515aa1616ebc3cecfc Mon Sep 17 00:00:00 2001 From: Riley Jones Date: Wed, 24 May 2023 16:48:41 +0000 Subject: [PATCH 1/4] update the runs table to read columns from redux state --- .../views/card_renderer/scalar_card_types.ts | 5 + tensorboard/webapp/runs/actions/BUILD | 1 + .../webapp/runs/actions/runs_actions.ts | 17 +++ .../webapp/runs/effects/runs_effects.ts | 2 +- tensorboard/webapp/runs/store/BUILD | 5 + .../webapp/runs/store/runs_reducers.ts | 39 ++++++- .../webapp/runs/store/runs_reducers_test.ts | 102 +++++++++++++++++- .../webapp/runs/store/runs_selectors.ts | 21 ++++ .../webapp/runs/store/runs_selectors_test.ts | 63 +++++++++++ tensorboard/webapp/runs/store/runs_types.ts | 3 + tensorboard/webapp/runs/store/testing.ts | 7 ++ .../webapp/runs/views/runs_table/BUILD | 3 + .../views/runs_table/runs_table_container.ts | 66 +++++++----- .../runs/views/runs_table/runs_table_test.ts | 12 +++ tensorboard/webapp/testing/BUILD | 1 + tensorboard/webapp/testing/utils.ts | 8 +- .../webapp/widgets/data_table/types.ts | 1 + 17 files changed, 328 insertions(+), 28 deletions(-) diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_types.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_types.ts index a901e61c9e..c6bd6d3dd1 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_types.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_types.ts @@ -92,3 +92,8 @@ export type MinMaxStep = { minStep: number; maxStep: number; }; + +export type RunToHParamValues = Record< + string, + Record +>; diff --git a/tensorboard/webapp/runs/actions/BUILD b/tensorboard/webapp/runs/actions/BUILD index d5b90c2047..dd325d1af3 100644 --- a/tensorboard/webapp/runs/actions/BUILD +++ b/tensorboard/webapp/runs/actions/BUILD @@ -14,6 +14,7 @@ tf_ts_library( "//tensorboard/webapp/runs:types", "//tensorboard/webapp/runs/data_source", "//tensorboard/webapp/types:ui", + "//tensorboard/webapp/widgets/data_table:types", "@npm//@ngrx/store", ], ) diff --git a/tensorboard/webapp/runs/actions/runs_actions.ts b/tensorboard/webapp/runs/actions/runs_actions.ts index 8a85a1a491..79d08c6c20 100644 --- a/tensorboard/webapp/runs/actions/runs_actions.ts +++ b/tensorboard/webapp/runs/actions/runs_actions.ts @@ -20,6 +20,7 @@ import {createAction, props} from '@ngrx/store'; import {SortDirection} from '../../types/ui'; import {Run} from '../data_source/runs_data_source_types'; import {ExperimentIdToRunsAndMetadata, GroupBy, SortKey} from '../types'; +import {ColumnHeader, SortingInfo} from '../../widgets/data_table/types'; /** * The action can fire when no requests are actually made (i.e., an empty @@ -96,3 +97,19 @@ export const runGroupByChanged = createAction( '[Runs] Run Group By Changed', props<{experimentIds: string[]; groupBy: GroupBy}>() ); + +/** + * Inserts the provided column header at the specified index. + */ +export const runsTableHeaderAdded = createAction( + '[Runs] Runs Table Header Added', + props<{header: ColumnHeader; index?: number}>() +); + +/** + * Updates the sorting logic used by the runs data tabe. + */ +export const runsTableSortingInfoChanged = createAction( + '[Runs] Runs Table Sorting Info Changed', + props<{sortingInfo: SortingInfo}>() +); diff --git a/tensorboard/webapp/runs/effects/runs_effects.ts b/tensorboard/webapp/runs/effects/runs_effects.ts index ca3e952c8f..52d0b7bd18 100644 --- a/tensorboard/webapp/runs/effects/runs_effects.ts +++ b/tensorboard/webapp/runs/effects/runs_effects.ts @@ -205,7 +205,7 @@ export class RunsEffects { }), map((runsAndMedataList) => { const newRunsAndMetadata = {} as ExperimentIdToRunsAndMetadata; - const runsForAllExperiments = []; + const runsForAllExperiments: Run[] = []; for (const runsAndMedata of runsAndMedataList) { runsForAllExperiments.push(...runsAndMedata.runs); diff --git a/tensorboard/webapp/runs/store/BUILD b/tensorboard/webapp/runs/store/BUILD index 810cfb745a..3f4c3fe25d 100644 --- a/tensorboard/webapp/runs/store/BUILD +++ b/tensorboard/webapp/runs/store/BUILD @@ -22,6 +22,7 @@ tf_ts_library( "//tensorboard/webapp/types", "//tensorboard/webapp/types:ui", "//tensorboard/webapp/util:ngrx", + "//tensorboard/webapp/widgets/data_table:types", "@npm//@ngrx/store", ], ) @@ -47,6 +48,7 @@ tf_ts_library( "//tensorboard/webapp/runs:types", "//tensorboard/webapp/types", "//tensorboard/webapp/types:ui", + "//tensorboard/webapp/widgets/data_table:types", "@npm//@ngrx/store", ], ) @@ -62,6 +64,7 @@ tf_ts_library( "//tensorboard/webapp/runs/data_source", "//tensorboard/webapp/types", "//tensorboard/webapp/types:ui", + "//tensorboard/webapp/widgets/data_table:types", ], ) @@ -74,6 +77,7 @@ tf_ts_library( "//tensorboard/webapp/runs:types", "//tensorboard/webapp/runs/data_source", "//tensorboard/webapp/types:ui", + "//tensorboard/webapp/widgets/data_table:types", ], ) @@ -101,6 +105,7 @@ tf_ts_library( "//tensorboard/webapp/testing:lang", "//tensorboard/webapp/types", "//tensorboard/webapp/types:ui", + "//tensorboard/webapp/widgets/data_table:types", "@npm//@types/jasmine", ], ) diff --git a/tensorboard/webapp/runs/store/runs_reducers.ts b/tensorboard/webapp/runs/store/runs_reducers.ts index 28fd55fa06..77cb047e25 100644 --- a/tensorboard/webapp/runs/store/runs_reducers.ts +++ b/tensorboard/webapp/runs/store/runs_reducers.ts @@ -35,9 +35,11 @@ import { RunsDataState, RunsState, RunsUiNamespacedState, + RunsUiNonNamespacedState, RunsUiState, } from './runs_types'; import {createGroupBy, groupRuns} from './utils'; +import {ColumnHeaderType, SortingOrder} from '../../widgets/data_table/types'; const { initialState: dataInitialState, @@ -309,7 +311,10 @@ const initialSort: RunsUiNamespacedState['sort'] = { direction: SortDirection.UNSET, }; const {initialState: uiInitialState, reducers: uiNamespaceContextedReducers} = - createNamespaceContextedState( + createNamespaceContextedState< + RunsUiNamespacedState, + RunsUiNonNamespacedState + >( { paginationOption: { pageIndex: 0, @@ -317,6 +322,19 @@ const {initialState: uiInitialState, reducers: uiNamespaceContextedReducers} = }, sort: initialSort, selectionState: new Map(), + runsTableHeaders: [ + { + type: ColumnHeaderType.RUN, + name: 'run', + displayName: 'Run', + enabled: true, + }, + ], + sortingInfo: { + header: ColumnHeaderType.RUN, + name: 'run', + order: SortingOrder.DESCENDING, + }, }, {} ); @@ -407,6 +425,25 @@ const uiReducer: ActionReducer = createReducer( ...state, selectionState: nextSelectionState, }; + }), + on(runsActions.runsTableHeaderAdded, (state, {header, index}) => { + const newRunsTableHeaders = [...state.runsTableHeaders]; + if (index === undefined) { + newRunsTableHeaders.push(header); + } else { + newRunsTableHeaders.splice(index, 0, header); + } + + return { + ...state, + runsTableHeaders: newRunsTableHeaders, + }; + }), + on(runsActions.runsTableSortingInfoChanged, (state, {sortingInfo}) => { + return { + ...state, + sortingInfo, + }; }) ); diff --git a/tensorboard/webapp/runs/store/runs_reducers_test.ts b/tensorboard/webapp/runs/store/runs_reducers_test.ts index 993dc74b63..0dd86ab0f3 100644 --- a/tensorboard/webapp/runs/store/runs_reducers_test.ts +++ b/tensorboard/webapp/runs/store/runs_reducers_test.ts @@ -22,11 +22,12 @@ import {RouteKind} from '../../app_routing/types'; import {deepFreeze} from '../../testing/lang'; import {DataLoadState} from '../../types/data'; import {SortDirection} from '../../types/ui'; +import {ColumnHeaderType, SortingOrder} from '../../widgets/data_table/types'; import * as actions from '../actions'; import {buildHparamsAndMetadata} from '../data_source/testing'; import {GroupByKey, SortType, URLDeserializedState} from '../types'; import * as runsReducers from './runs_reducers'; -import {MAX_NUM_RUNS_TO_ENABLE_BY_DEFAULT, Run} from './runs_types'; +import {MAX_NUM_RUNS_TO_ENABLE_BY_DEFAULT, Run, RunsState} from './runs_types'; import {buildRun, buildRunsState} from './testing'; describe('runs_reducers', () => { @@ -1353,4 +1354,103 @@ describe('runs_reducers', () => { expect(nextState.data.initialGroupBy.key).toBe(GroupByKey.RUN); }); }); + + describe('runsTableHeaderAdded', () => { + let state: RunsState; + + beforeEach(() => { + state = buildRunsState( + {}, + { + runsTableHeaders: [ + { + type: ColumnHeaderType.RUN, + name: 'run', + displayName: 'Run', + enabled: true, + }, + { + type: ColumnHeaderType.VALUE, + name: 'value', + displayName: 'Value', + enabled: true, + }, + ], + } + ); + }); + + it('adds new column to end of list when no index is provided', () => { + const nextState = runsReducers.reducers( + state, + actions.runsTableHeaderAdded({ + header: { + type: ColumnHeaderType.COLOR, + name: 'color', + displayName: 'Color', + enabled: true, + }, + }) + ); + expect( + nextState.ui.runsTableHeaders.map((header) => header.type) + ).toEqual([ + ColumnHeaderType.RUN, + ColumnHeaderType.VALUE, + ColumnHeaderType.COLOR, + ]); + }); + + it('adds new column at the specified index', () => { + const nextState = runsReducers.reducers( + state, + actions.runsTableHeaderAdded({ + header: { + type: ColumnHeaderType.COLOR, + name: 'color', + displayName: 'Color', + enabled: true, + }, + index: 1, + }) + ); + expect( + nextState.ui.runsTableHeaders.map((header) => header.type) + ).toEqual([ + ColumnHeaderType.RUN, + ColumnHeaderType.COLOR, + ColumnHeaderType.VALUE, + ]); + }); + }); + + describe('runsTableSortingInfoChanged', () => { + it('it returns the current runs table sorting info', () => { + const state = buildRunsState( + {}, + { + sortingInfo: { + header: ColumnHeaderType.RUN, + name: 'run', + order: SortingOrder.ASCENDING, + }, + } + ); + const nextState = runsReducers.reducers( + state, + actions.runsTableSortingInfoChanged({ + sortingInfo: { + header: ColumnHeaderType.HPARAM, + name: 'lr', + order: SortingOrder.DESCENDING, + }, + }) + ); + expect(nextState.ui.sortingInfo).toEqual({ + header: ColumnHeaderType.HPARAM, + name: 'lr', + order: SortingOrder.DESCENDING, + }); + }); + }); }); diff --git a/tensorboard/webapp/runs/store/runs_selectors.ts b/tensorboard/webapp/runs/store/runs_selectors.ts index 23ceea336d..5dc28ffd01 100644 --- a/tensorboard/webapp/runs/store/runs_selectors.ts +++ b/tensorboard/webapp/runs/store/runs_selectors.ts @@ -26,6 +26,7 @@ import { RUNS_FEATURE_KEY, } from './runs_types'; import {createGroupBy} from './utils'; +import {ColumnHeader, SortingInfo} from '../../widgets/data_table/types'; const getRunsState = createFeatureSelector(RUNS_FEATURE_KEY); @@ -236,3 +237,23 @@ export const getColorGroupRegexString = createSelector( return state.colorGroupRegexString; } ); + +/** + * Gets the columns to be displayed by the runs table. + */ +export const getRunsTableHeaders = createSelector( + getUiState, + (state: RunsUiState): ColumnHeader[] => { + return state.runsTableHeaders; + } +); + +/** + * Gets the information needed to sort the runs data table. + */ +export const getRunsTableSortingInfo = createSelector( + getUiState, + (state: RunsUiState): SortingInfo => { + return state.sortingInfo; + } +); diff --git a/tensorboard/webapp/runs/store/runs_selectors_test.ts b/tensorboard/webapp/runs/store/runs_selectors_test.ts index 5b5797544a..0671fd136a 100644 --- a/tensorboard/webapp/runs/store/runs_selectors_test.ts +++ b/tensorboard/webapp/runs/store/runs_selectors_test.ts @@ -14,6 +14,7 @@ limitations under the License. ==============================================================================*/ import {DataLoadState} from '../../types/data'; import {SortDirection} from '../../types/ui'; +import {ColumnHeaderType, SortingOrder} from '../../widgets/data_table/types'; import {GroupByKey, SortType} from '../types'; import * as selectors from './runs_selectors'; import {buildRun, buildRunsState, buildStateFromRunsState} from './testing'; @@ -584,4 +585,66 @@ describe('runs_selectors', () => { expect(selectors.getColorGroupRegexString(state)).toEqual('foo(\\d+)'); }); }); + + describe('#getRunsTableHeaders', () => { + it('returns the runs table headers', () => { + const state = buildStateFromRunsState( + buildRunsState( + {}, + { + runsTableHeaders: [ + { + type: ColumnHeaderType.RUN, + name: 'run', + displayName: 'Run', + enabled: true, + }, + { + type: ColumnHeaderType.COLOR, + name: 'color', + displayName: 'Color', + enabled: false, + }, + ], + } + ) + ); + expect(selectors.getRunsTableHeaders(state)).toEqual([ + { + type: ColumnHeaderType.RUN, + name: 'run', + displayName: 'Run', + enabled: true, + }, + { + type: ColumnHeaderType.COLOR, + name: 'color', + displayName: 'Color', + enabled: false, + }, + ]); + }); + }); + + describe('#getRunsTableSortingInfo', () => { + it('returns the runs data table sorting info', () => { + const state = buildStateFromRunsState( + buildRunsState( + {}, + { + sortingInfo: { + header: ColumnHeaderType.RUN, + name: 'run', + order: SortingOrder.ASCENDING, + }, + } + ) + ); + expect(selectors.getRunsTableSortingInfo(state)).toEqual({ + header: ColumnHeaderType.RUN, + name: 'run', + order: SortingOrder.ASCENDING, + }); + }); + }); }); diff --git a/tensorboard/webapp/runs/store/runs_types.ts b/tensorboard/webapp/runs/store/runs_types.ts index 0bfa2b34d3..f170af7f82 100644 --- a/tensorboard/webapp/runs/store/runs_types.ts +++ b/tensorboard/webapp/runs/store/runs_types.ts @@ -19,6 +19,7 @@ limitations under the License. import {NamespaceContextedState} from '../../app_routing/namespaced_state_reducer_helper'; import {LoadState} from '../../types/data'; import {SortDirection} from '../../types/ui'; +import {ColumnHeader, SortingInfo} from '../../widgets/data_table/types'; import {HparamValue} from '../data_source/runs_data_source_types'; import {GroupBy, GroupByKey, SortKey} from '../types'; @@ -79,6 +80,8 @@ export interface RunsUiNamespacedState { * Indicates whether the run is selected. */ selectionState: Map; + runsTableHeaders: ColumnHeader[]; + sortingInfo: SortingInfo; } export interface RunsUiNonNamespacedState {} diff --git a/tensorboard/webapp/runs/store/testing.ts b/tensorboard/webapp/runs/store/testing.ts index 0b1f1fbfbd..3d8839c1eb 100644 --- a/tensorboard/webapp/runs/store/testing.ts +++ b/tensorboard/webapp/runs/store/testing.ts @@ -17,6 +17,7 @@ limitations under the License. */ import {SortDirection} from '../../types/ui'; +import {ColumnHeaderType, SortingOrder} from '../../widgets/data_table/types'; import {GroupByKey} from '../types'; import { Run, @@ -68,6 +69,12 @@ export function buildRunsState( paginationOption: {pageIndex: 0, pageSize: 0}, sort: {key: null, direction: SortDirection.UNSET}, selectionState: new Map(), + runsTableHeaders: [], + sortingInfo: { + header: ColumnHeaderType.RUN, + name: 'run', + order: SortingOrder.DESCENDING, + }, ...uiOverride, }, }; diff --git a/tensorboard/webapp/runs/views/runs_table/BUILD b/tensorboard/webapp/runs/views/runs_table/BUILD index 3a5a0c619f..e5dfe35002 100644 --- a/tensorboard/webapp/runs/views/runs_table/BUILD +++ b/tensorboard/webapp/runs/views/runs_table/BUILD @@ -85,6 +85,8 @@ tf_ng_module( "//tensorboard/webapp/feature_flag/store", "//tensorboard/webapp/hparams", "//tensorboard/webapp/hparams:types", + "//tensorboard/webapp/metrics/views/card_renderer:scalar_card_types", + "//tensorboard/webapp/metrics/views/main_view:common_selectors", "//tensorboard/webapp/runs:types", "//tensorboard/webapp/runs/actions", "//tensorboard/webapp/runs/data_source", @@ -155,6 +157,7 @@ tf_ts_library( "//tensorboard/webapp/types", "//tensorboard/webapp/types:ui", "//tensorboard/webapp/widgets/data_table", + "//tensorboard/webapp/widgets/data_table:types", "//tensorboard/webapp/widgets/experiment_alias", "//tensorboard/webapp/widgets/filter_input", "//tensorboard/webapp/widgets/range_input", diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts index 566b0f248e..71b88ddb3a 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts @@ -57,6 +57,8 @@ import { getRunSelectorRegexFilter, getRunSelectorSort, getRunsLoadState, + getRunsTableHeaders, + getRunsTableSortingInfo, } from '../../../selectors'; import {DataLoadState, LoadState} from '../../../types/data'; import {SortDirection} from '../../../types/ui'; @@ -64,7 +66,7 @@ import {matchRunToRegex} from '../../../util/matcher'; import {getEnableHparamsInTimeSeries} from '../../../feature_flag/store/feature_flag_selectors'; import { ColumnHeaderType, - SortingOrder, + SortingInfo, TableData, } from '../../../widgets/data_table/types'; import { @@ -75,6 +77,7 @@ import { runSelectorRegexFilterChanged, runSelectorSortChanged, runTableShown, + runsTableSortingInfoChanged, singleRunSelected, } from '../../actions'; import {MAX_NUM_RUNS_TO_ENABLE_BY_DEFAULT} from '../../store/runs_types'; @@ -85,6 +88,8 @@ import { MetricColumn, } from './runs_table_component'; import {RunsTableColumn, RunTableItem} from './types'; +import {getFilteredRenderableRunsFromRoute} from '../../../metrics/views/main_view/common_selectors'; +import {RunToHParamValues} from '../../../metrics/views/card_renderer/scalar_card_types'; const getRunsLoading = createSelector< State, @@ -229,10 +234,10 @@ function matchFilter( > runs-table-component { width: 100%; } + + :host.flex-layout > tb-data-table { + overflow-y: scroll; + width: 100%; + } `, ], changeDetection: ChangeDetectionStrategy.OnPush, @@ -262,22 +272,7 @@ export class RunsTableContainer implements OnInit, OnDestroy { allItemsLength$?: Observable; pageItems$?: Observable; numSelectedItems$?: Observable; - - // TODO(jameshollyer): Move these values to ngrx and make these Observables. - runsColumns = [ - { - type: ColumnHeaderType.RUN, - name: 'run', - displayName: 'Run', - enabled: true, - }, - ]; - sortingInfo = { - header: ColumnHeaderType.RUN, - name: 'run', - order: SortingOrder.ASCENDING, - }; - columnCustomizationEnabled = true; + sortingInfo$ = this.store.select(getRunsTableSortingInfo); smoothingEnabled = false; hparamColumns$: Observable = of([]); @@ -311,6 +306,19 @@ export class RunsTableContainer implements OnInit, OnDestroy { paginationOption$ = this.store.select(getRunSelectorPaginationOption); regexFilter$ = this.store.select(getRunSelectorRegexFilter); HParamsEnabled = new BehaviorSubject(false); + runsColumns$ = this.store.select(getRunsTableHeaders); + + runToHParamValues$ = this.store + .select(getFilteredRenderableRunsFromRoute) + .pipe( + map((items) => { + return items.reduce((map, item) => { + map[item.run.id] = Object.fromEntries(item.hparams.entries()); + return map; + }, {} as RunToHParamValues); + }) + ); + private readonly ngUnsubscribe = new Subject(); constructor(private readonly store: Store) {} @@ -335,8 +343,7 @@ export class RunsTableContainer implements OnInit, OnDestroy { this.allRunsTableData$ = combineLatest(getRunTableDataPerExperiment$).pipe( map((itemsForExperiments: TableData[][]) => { - const items = [] as TableData[]; - return items.concat(...itemsForExperiments); + return itemsForExperiments.flat(); }) ); @@ -575,18 +582,25 @@ export class RunsTableContainer implements OnInit, OnDestroy { return combineLatest([ this.store.select(getRuns, {experimentId}), this.store.select(getRunColorMap), + this.runsColumns$, + this.runToHParamValues$, ]).pipe( - map(([runs, colorMap]) => { + map(([runs, colorMap, runsColumns, runToHParamValues]) => { return runs.map((run) => { const tableData: TableData = { id: run.id, color: colorMap[run.id], }; - this.runsColumns.forEach((column) => { + runsColumns.forEach((column) => { switch (column.type) { case ColumnHeaderType.RUN: tableData[column.name!] = run.name; break; + case ColumnHeaderType.HPARAM: + tableData[column.name] = runToHParamValues[run.id]?.[ + column.name + ] as string | number; + break; default: break; } @@ -597,6 +611,10 @@ export class RunsTableContainer implements OnInit, OnDestroy { ); } + sortDataBy(sortingInfo: SortingInfo) { + this.store.dispatch(runsTableSortingInfoChanged({sortingInfo})); + } + private getRunTableItemsForExperiment( experimentId: string ): Observable { diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts index 57509b353f..4335ebeca5 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts @@ -72,6 +72,7 @@ import { getRunSelectorRegexFilter, getRunSelectorSort, getRunsLoadState, + getRunsTableHeaders, } from '../../../selectors'; import {selectors as settingsSelectors} from '../../../settings'; import {buildColorPalette} from '../../../settings/testing'; @@ -105,6 +106,7 @@ import {RunsGroupMenuButtonContainer} from './runs_group_menu_button_container'; import {RunsTableComponent} from './runs_table_component'; import {RunsTableContainer, TEST_ONLY} from './runs_table_container'; import {HparamSpec, MetricSpec, RunsTableColumn} from './types'; +import {ColumnHeaderType} from '../../../widgets/data_table/types'; @Injectable() class ColorPickerTestHelper { @@ -3203,6 +3205,16 @@ describe('runs_table', () => { buildRun({id: 'book2', name: 'The Chamber Of Secrets'}), ]) ); + selectSpy.withArgs(getRunsTableHeaders).and.returnValue( + of([ + { + type: ColumnHeaderType.RUN, + name: 'run', + displayName: 'Run', + enabled: true, + }, + ]) + ); store.overrideSelector(getRunColorMap, { book1: '#000', diff --git a/tensorboard/webapp/testing/BUILD b/tensorboard/webapp/testing/BUILD index a76a1779cb..a2b1f456ea 100644 --- a/tensorboard/webapp/testing/BUILD +++ b/tensorboard/webapp/testing/BUILD @@ -78,6 +78,7 @@ tf_ts_library( "//tensorboard/webapp/feature_flag/store:testing", "//tensorboard/webapp/feature_flag/store:types", "//tensorboard/webapp/hparams/_redux:testing", + "//tensorboard/webapp/hparams/_redux:types", "//tensorboard/webapp/metrics:test_lib", "//tensorboard/webapp/metrics/store", "//tensorboard/webapp/notification_center/_redux:testing", diff --git a/tensorboard/webapp/testing/utils.ts b/tensorboard/webapp/testing/utils.ts index 92793f723a..b86784f247 100644 --- a/tensorboard/webapp/testing/utils.ts +++ b/tensorboard/webapp/testing/utils.ts @@ -56,6 +56,7 @@ import { createState as createSettings, createSettingsState, } from '../settings/testing'; +import {HPARAMS_FEATURE_KEY} from '../hparams/_redux/types'; export function buildMockState(overrides: Partial = {}): State { return { @@ -81,7 +82,12 @@ export function buildMockState(overrides: Partial = {}): State { buildAppRoutingState(overrides[APP_ROUTING_FEATURE_KEY]) ), ...buildStateFromFeatureFlagsState(buildFeatureFlagState()), - ...buildStateFromHparamsState(buildHparamsState()), + ...buildStateFromHparamsState( + buildHparamsState( + overrides[HPARAMS_FEATURE_KEY]?.specs, + overrides[HPARAMS_FEATURE_KEY]?.filters + ) + ), ...buildStateFromNotificationState( buildNotificationState(overrides[NOTIFICATION_FEATURE_KEY] || {}) ), diff --git a/tensorboard/webapp/widgets/data_table/types.ts b/tensorboard/webapp/widgets/data_table/types.ts index f9c8884094..f17dcbb072 100644 --- a/tensorboard/webapp/widgets/data_table/types.ts +++ b/tensorboard/webapp/widgets/data_table/types.ts @@ -38,6 +38,7 @@ export enum ColumnHeaderType { STEP_AT_MIN = 'STEP_AT_MIN', MEAN = 'MEAN', RAW_CHANGE = 'RAW_CHANGE', + HPARAM = 'HPARAM', } export interface ColumnHeader { From 48bbc0eebd248e44daac8c6c256f306cac1b69b8 Mon Sep 17 00:00:00 2001 From: Riley Jones Date: Thu, 25 May 2023 17:14:29 +0000 Subject: [PATCH 2/4] address pr comments --- .../views/card_renderer/scalar_card_types.ts | 5 -- .../data_source/runs_data_source_types.ts | 5 ++ .../webapp/runs/effects/runs_effects.ts | 2 +- .../webapp/runs/store/runs_reducers_test.ts | 2 +- .../webapp/runs/views/runs_table/BUILD | 1 + .../views/runs_table/runs_table_container.ts | 11 ++-- .../runs/views/runs_table/runs_table_test.ts | 53 ++++++++++++++++++- tensorboard/webapp/testing/utils.ts | 8 +-- 8 files changed, 66 insertions(+), 21 deletions(-) diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_types.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_types.ts index c6bd6d3dd1..a901e61c9e 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_types.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_types.ts @@ -92,8 +92,3 @@ export type MinMaxStep = { minStep: number; maxStep: number; }; - -export type RunToHParamValues = Record< - string, - Record ->; diff --git a/tensorboard/webapp/runs/data_source/runs_data_source_types.ts b/tensorboard/webapp/runs/data_source/runs_data_source_types.ts index a62365dadc..1865c8cf04 100644 --- a/tensorboard/webapp/runs/data_source/runs_data_source_types.ts +++ b/tensorboard/webapp/runs/data_source/runs_data_source_types.ts @@ -86,3 +86,8 @@ export abstract class RunsDataSource { experimentId: string ): Observable; } + +export type RunToHParamValues = Record< + string, + Map +>; diff --git a/tensorboard/webapp/runs/effects/runs_effects.ts b/tensorboard/webapp/runs/effects/runs_effects.ts index 52d0b7bd18..ca3e952c8f 100644 --- a/tensorboard/webapp/runs/effects/runs_effects.ts +++ b/tensorboard/webapp/runs/effects/runs_effects.ts @@ -205,7 +205,7 @@ export class RunsEffects { }), map((runsAndMedataList) => { const newRunsAndMetadata = {} as ExperimentIdToRunsAndMetadata; - const runsForAllExperiments: Run[] = []; + const runsForAllExperiments = []; for (const runsAndMedata of runsAndMedataList) { runsForAllExperiments.push(...runsAndMedata.runs); diff --git a/tensorboard/webapp/runs/store/runs_reducers_test.ts b/tensorboard/webapp/runs/store/runs_reducers_test.ts index 0dd86ab0f3..e0761e36fb 100644 --- a/tensorboard/webapp/runs/store/runs_reducers_test.ts +++ b/tensorboard/webapp/runs/store/runs_reducers_test.ts @@ -1425,7 +1425,7 @@ describe('runs_reducers', () => { }); describe('runsTableSortingInfoChanged', () => { - it('it returns the current runs table sorting info', () => { + it('returns the current runs table sorting info', () => { const state = buildRunsState( {}, { diff --git a/tensorboard/webapp/runs/views/runs_table/BUILD b/tensorboard/webapp/runs/views/runs_table/BUILD index e5dfe35002..14c3ef93a7 100644 --- a/tensorboard/webapp/runs/views/runs_table/BUILD +++ b/tensorboard/webapp/runs/views/runs_table/BUILD @@ -138,6 +138,7 @@ tf_ts_library( "//tensorboard/webapp/angular:expect_angular_platform_browser_dynamic_testing", "//tensorboard/webapp/angular:expect_ngrx_store_testing", "//tensorboard/webapp/app_routing:testing", + "//tensorboard/webapp/metrics/views/main_view:common_selectors", "//tensorboard/webapp/app_routing:types", "//tensorboard/webapp/experiments/store:testing", "//tensorboard/webapp/feature_flag/store", diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts index 71b88ddb3a..952ff8175c 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts @@ -89,7 +89,7 @@ import { } from './runs_table_component'; import {RunsTableColumn, RunTableItem} from './types'; import {getFilteredRenderableRunsFromRoute} from '../../../metrics/views/main_view/common_selectors'; -import {RunToHParamValues} from '../../../metrics/views/card_renderer/scalar_card_types'; +import {RunToHParamValues} from '../../data_source/runs_data_source_types'; const getRunsLoading = createSelector< State, @@ -238,7 +238,7 @@ function matchFilter( [data]="allRunsTableData$ | async" [sortingInfo]="sortingInfo$ | async" columnCustomizationEnabled="true" - [smoothingEnabled]="smoothingEnabled" + smoothingEnabled="false" (sortDataBy)="sortDataBy($event)" (orderColumns)="orderColumns($event)" > @@ -273,7 +273,6 @@ export class RunsTableContainer implements OnInit, OnDestroy { pageItems$?: Observable; numSelectedItems$?: Observable; sortingInfo$ = this.store.select(getRunsTableSortingInfo); - smoothingEnabled = false; hparamColumns$: Observable = of([]); metricColumns$: Observable = of([]); @@ -313,7 +312,7 @@ export class RunsTableContainer implements OnInit, OnDestroy { .pipe( map((items) => { return items.reduce((map, item) => { - map[item.run.id] = Object.fromEntries(item.hparams.entries()); + map[item.run.id] = item.hparams; return map; }, {} as RunToHParamValues); }) @@ -597,9 +596,9 @@ export class RunsTableContainer implements OnInit, OnDestroy { tableData[column.name!] = run.name; break; case ColumnHeaderType.HPARAM: - tableData[column.name] = runToHParamValues[run.id]?.[ + tableData[column.name] = runToHParamValues[run.id]?.get( column.name - ] as string | number; + ) as string | number; break; default: break; diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts index 4335ebeca5..2697f4b796 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts @@ -105,8 +105,9 @@ import {RunsGroupMenuButtonComponent} from './runs_group_menu_button_component'; import {RunsGroupMenuButtonContainer} from './runs_group_menu_button_container'; import {RunsTableComponent} from './runs_table_component'; import {RunsTableContainer, TEST_ONLY} from './runs_table_container'; -import {HparamSpec, MetricSpec, RunsTableColumn} from './types'; +import {HparamSpec, MetricSpec, RunTableItem, RunsTableColumn} from './types'; import {ColumnHeaderType} from '../../../widgets/data_table/types'; +import {getFilteredRenderableRunsFromRoute} from '../../../metrics/views/main_view/common_selectors'; @Injectable() class ColorPickerTestHelper { @@ -3232,5 +3233,55 @@ describe('runs_table', () => { {id: 'book2', color: '#111', run: 'The Chamber Of Secrets'}, ]); }); + + it('passes hparam values to data table', () => { + const run1 = buildRun({id: 'book1', name: "The Philosopher's Stone"}); + const run2 = buildRun({id: 'book2', name: 'The Chamber Of Secrets'}); + // To make sure we only return the runs when called with the right props. + const selectSpy = spyOn(store, 'select').and.callThrough(); + selectSpy + .withArgs(getRuns, {experimentId: 'book'}) + .and.returnValue(of([run1, run2])); + + selectSpy.withArgs(getRunsTableHeaders).and.returnValue( + of([ + { + type: ColumnHeaderType.HPARAM, + name: 'batch_size', + displayName: 'Batch Size', + enabled: true, + }, + ]) + ); + + selectSpy.withArgs(getFilteredRenderableRunsFromRoute).and.returnValue( + of([ + { + run: run1, + hparams: new Map([['batch_size', 1]]), + } as RunTableItem, + { + run: run2, + hparams: new Map([['batch_size', 2]]), + } as RunTableItem, + ]) + ); + + store.overrideSelector(getRunColorMap, { + book1: '#000', + book2: '#111', + }); + + const fixture = createComponent(['book']); + fixture.detectChanges(); + const dataTableComponent = fixture.debugElement.query( + By.directive(DataTableComponent) + ); + + expect(dataTableComponent.componentInstance.data).toEqual([ + {id: 'book1', color: '#000', batch_size: 1}, + {id: 'book2', color: '#111', batch_size: 2}, + ]); + }); }); }); diff --git a/tensorboard/webapp/testing/utils.ts b/tensorboard/webapp/testing/utils.ts index b86784f247..92793f723a 100644 --- a/tensorboard/webapp/testing/utils.ts +++ b/tensorboard/webapp/testing/utils.ts @@ -56,7 +56,6 @@ import { createState as createSettings, createSettingsState, } from '../settings/testing'; -import {HPARAMS_FEATURE_KEY} from '../hparams/_redux/types'; export function buildMockState(overrides: Partial = {}): State { return { @@ -82,12 +81,7 @@ export function buildMockState(overrides: Partial = {}): State { buildAppRoutingState(overrides[APP_ROUTING_FEATURE_KEY]) ), ...buildStateFromFeatureFlagsState(buildFeatureFlagState()), - ...buildStateFromHparamsState( - buildHparamsState( - overrides[HPARAMS_FEATURE_KEY]?.specs, - overrides[HPARAMS_FEATURE_KEY]?.filters - ) - ), + ...buildStateFromHparamsState(buildHparamsState()), ...buildStateFromNotificationState( buildNotificationState(overrides[NOTIFICATION_FEATURE_KEY] || {}) ), From 13e3ba550bdbdd088673fe60e4e5e00407cd4c7c Mon Sep 17 00:00:00 2001 From: Riley Jones Date: Thu, 25 May 2023 17:18:20 +0000 Subject: [PATCH 3/4] remove unused dependency --- tensorboard/webapp/runs/views/runs_table/BUILD | 1 - tensorboard/webapp/runs/views/runs_table/runs_table_test.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tensorboard/webapp/runs/views/runs_table/BUILD b/tensorboard/webapp/runs/views/runs_table/BUILD index 14c3ef93a7..9dc69bb5b9 100644 --- a/tensorboard/webapp/runs/views/runs_table/BUILD +++ b/tensorboard/webapp/runs/views/runs_table/BUILD @@ -85,7 +85,6 @@ tf_ng_module( "//tensorboard/webapp/feature_flag/store", "//tensorboard/webapp/hparams", "//tensorboard/webapp/hparams:types", - "//tensorboard/webapp/metrics/views/card_renderer:scalar_card_types", "//tensorboard/webapp/metrics/views/main_view:common_selectors", "//tensorboard/webapp/runs:types", "//tensorboard/webapp/runs/actions", diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts index 2697f4b796..cfef3bac89 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts @@ -1866,7 +1866,7 @@ describe('runs_table', () => { describe('"too many runs" alert', () => { function createRuns(runCount: number): Run[] { - const runs = []; + const runs: Run[] = []; for (let i = 0; i < runCount; i++) { runs.push( buildRun({ From 5a53cc23b0abb8112636ebbe3cc7ca47ea9dca43 Mon Sep 17 00:00:00 2001 From: Riley Jones Date: Thu, 25 May 2023 18:25:36 +0000 Subject: [PATCH 4/4] reformat build file --- tensorboard/webapp/runs/views/runs_table/BUILD | 2 +- tensorboard/webapp/testing/BUILD | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tensorboard/webapp/runs/views/runs_table/BUILD b/tensorboard/webapp/runs/views/runs_table/BUILD index 9dc69bb5b9..886bf89ed8 100644 --- a/tensorboard/webapp/runs/views/runs_table/BUILD +++ b/tensorboard/webapp/runs/views/runs_table/BUILD @@ -137,13 +137,13 @@ tf_ts_library( "//tensorboard/webapp/angular:expect_angular_platform_browser_dynamic_testing", "//tensorboard/webapp/angular:expect_ngrx_store_testing", "//tensorboard/webapp/app_routing:testing", - "//tensorboard/webapp/metrics/views/main_view:common_selectors", "//tensorboard/webapp/app_routing:types", "//tensorboard/webapp/experiments/store:testing", "//tensorboard/webapp/feature_flag/store", "//tensorboard/webapp/hparams", "//tensorboard/webapp/hparams:testing", "//tensorboard/webapp/hparams:types", + "//tensorboard/webapp/metrics/views/main_view:common_selectors", "//tensorboard/webapp/runs:types", "//tensorboard/webapp/runs/actions", "//tensorboard/webapp/runs/data_source", diff --git a/tensorboard/webapp/testing/BUILD b/tensorboard/webapp/testing/BUILD index a2b1f456ea..a76a1779cb 100644 --- a/tensorboard/webapp/testing/BUILD +++ b/tensorboard/webapp/testing/BUILD @@ -78,7 +78,6 @@ tf_ts_library( "//tensorboard/webapp/feature_flag/store:testing", "//tensorboard/webapp/feature_flag/store:types", "//tensorboard/webapp/hparams/_redux:testing", - "//tensorboard/webapp/hparams/_redux:types", "//tensorboard/webapp/metrics:test_lib", "//tensorboard/webapp/metrics/store", "//tensorboard/webapp/notification_center/_redux:testing",