Skip to content

Commit 78b36f8

Browse files
hoonjihoonji
authored
Makes runs table use hparam store for hparam columns (#6736)
## Motivation for features / changes To display shared hparam columns across runs and scalar tables, both should read this data from the same hparam store. This PR first applies this change to the runs table. ## Technical description of changes Updates runs table to use columns from the hparam store, and to dispatch hparam actions misc: - Loads hparams from local storage on persistentSettingsLoaded - Explicitly sets runs table header context menu options in selector (they will differ for scalar tables) - Shows "hidden" (i.e. disabled) columns in the runs table (hide will only apply to scalar tables) ## Screenshots of UI changes (or N/A) UI works the same ## Detailed steps to verify changes work correctly (as executed by you) Manually verified that adding/removing/filter columns is unchanged
1 parent 0d76a12 commit 78b36f8

File tree

11 files changed

+185
-58
lines changed

11 files changed

+185
-58
lines changed

‎tensorboard/webapp/hparams/_redux/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ tf_ts_library(
6565
":types",
6666
":utils",
6767
"//tensorboard/webapp/hparams:_types",
68+
"//tensorboard/webapp/persistent_settings",
6869
"//tensorboard/webapp/runs/actions",
6970
"//tensorboard/webapp/widgets/data_table:types",
7071
"//tensorboard/webapp/widgets/data_table:utils",
@@ -168,6 +169,7 @@ tf_ts_library(
168169
"//tensorboard/webapp/app_routing/actions",
169170
"//tensorboard/webapp/core/actions",
170171
"//tensorboard/webapp/hparams:types",
172+
"//tensorboard/webapp/persistent_settings",
171173
"//tensorboard/webapp/runs/actions",
172174
"//tensorboard/webapp/runs/data_source:testing",
173175
"//tensorboard/webapp/runs/store:testing",

‎tensorboard/webapp/hparams/_redux/hparams_reducers.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@ See the License for the specific language governing permissions and
1313
limitations under the License.
1414
==============================================================================*/
1515
import {Action, ActionReducer, createReducer, on} from '@ngrx/store';
16-
import {Side} from '../../widgets/data_table/types';
1716
import {DataTableUtils} from '../../widgets/data_table/utils';
17+
import {persistentSettingsLoaded} from '../../persistent_settings';
18+
import {Side} from '../../widgets/data_table/types';
1819
import * as actions from './hparams_actions';
1920
import {HparamsState} from './types';
2021

@@ -33,6 +34,16 @@ const initialState: HparamsState = {
3334

3435
const reducer: ActionReducer<HparamsState, Action> = createReducer(
3536
initialState,
37+
on(persistentSettingsLoaded, (state, {partialSettings}) => {
38+
const {dashboardDisplayedHparamColumns: storedColumns} = partialSettings;
39+
if (storedColumns) {
40+
return {
41+
...state,
42+
dashboardDisplayedHparamColumns: storedColumns,
43+
};
44+
}
45+
return state;
46+
}),
3647
on(actions.hparamsFetchSessionGroupsSucceeded, (state, action) => {
3748
const nextDashboardSpecs = action.hparamsAndMetricsSpecs;
3849
const nextDashboardSessionGroups = action.sessionGroups;

‎tensorboard/webapp/hparams/_redux/hparams_reducers_test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,81 @@ import {reducers} from './hparams_reducers';
1919
import {buildHparamSpec, buildHparamsState, buildMetricSpec} from './testing';
2020
import {ColumnHeaderType, Side} from '../../widgets/data_table/types';
2121
import {DataTableUtils} from '../../widgets/data_table/utils';
22+
import {persistentSettingsLoaded} from '../../persistent_settings';
2223

2324
describe('hparams/_redux/hparams_reducers_test', () => {
25+
describe('#persistentSettingsLoaded', () => {
26+
it('loads dashboardDisplayedHparamColumns from the persistent settings storage', () => {
27+
const state = buildHparamsState({
28+
dashboardDisplayedHparamColumns: [],
29+
});
30+
const state2 = reducers(
31+
state,
32+
persistentSettingsLoaded({
33+
partialSettings: {
34+
dashboardDisplayedHparamColumns: [
35+
{
36+
type: ColumnHeaderType.HPARAM,
37+
name: 'conv_layers',
38+
displayName: 'Conv Layers',
39+
enabled: true,
40+
},
41+
{
42+
type: ColumnHeaderType.HPARAM,
43+
name: 'conv_kernel_size',
44+
displayName: 'Conv Kernel Size',
45+
enabled: true,
46+
},
47+
],
48+
},
49+
})
50+
);
51+
52+
expect(state2.dashboardDisplayedHparamColumns).toEqual([
53+
{
54+
type: ColumnHeaderType.HPARAM,
55+
name: 'conv_layers',
56+
displayName: 'Conv Layers',
57+
enabled: true,
58+
},
59+
{
60+
type: ColumnHeaderType.HPARAM,
61+
name: 'conv_kernel_size',
62+
displayName: 'Conv Kernel Size',
63+
enabled: true,
64+
},
65+
]);
66+
});
67+
68+
it('does nothing if persistent settings does not contain dashboardDisplayedHparamColumns', () => {
69+
const state = buildHparamsState({
70+
dashboardDisplayedHparamColumns: [
71+
{
72+
type: ColumnHeaderType.HPARAM,
73+
name: 'conv_layers',
74+
displayName: 'Conv Layers',
75+
enabled: true,
76+
},
77+
],
78+
});
79+
const state2 = reducers(
80+
state,
81+
persistentSettingsLoaded({
82+
partialSettings: {},
83+
})
84+
);
85+
86+
expect(state2.dashboardDisplayedHparamColumns).toEqual([
87+
{
88+
type: ColumnHeaderType.HPARAM,
89+
name: 'conv_layers',
90+
displayName: 'Conv Layers',
91+
enabled: true,
92+
},
93+
]);
94+
});
95+
});
96+
2497
describe('hparamsFetchSessionGroupsSucceeded', () => {
2598
it('saves action.hparamsAndMetricsSpecs as dashboardSpecs', () => {
2699
const state = buildHparamsState({

‎tensorboard/webapp/runs/store/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ tf_ts_library(
9999
":testing",
100100
":types",
101101
":utils",
102+
"//tensorboard/webapp:app_state",
102103
"//tensorboard/webapp/app_routing:testing",
103104
"//tensorboard/webapp/app_routing:types",
104105
"//tensorboard/webapp/app_routing/actions",

‎tensorboard/webapp/runs/store/runs_selectors.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,16 @@ export const getRunsTableSortingInfo = createSelector(
330330
export const getGroupedRunsTableHeaders = createSelector(
331331
getRunsTableHeaders,
332332
getDashboardDisplayedHparamColumns,
333-
(runsTableHeaders, hparamColumns) =>
334-
DataTableUtils.groupColumns([...runsTableHeaders, ...hparamColumns])
333+
(runsTableHeaders, hparamColumns) => {
334+
// Override hparam options to match runs table requirements.
335+
const columns = [...runsTableHeaders, ...hparamColumns].map((column) => {
336+
const newColumn = {...column};
337+
if (column.type === 'HPARAM') {
338+
newColumn.removable = true;
339+
newColumn.hidable = false;
340+
}
341+
return newColumn;
342+
});
343+
return DataTableUtils.groupColumns(columns);
344+
}
335345
);

‎tensorboard/webapp/runs/store/runs_selectors_test.ts

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
buildHparamSpec,
2525
} from '../../hparams/testing';
2626
import {buildMockState} from '../../testing/utils';
27+
import {State} from '../../app_state';
2728
import {DataLoadState} from '../../types/data';
2829
import {ColumnHeaderType, SortingOrder} from '../../widgets/data_table/types';
2930
import {GroupByKey} from '../types';
@@ -1029,8 +1030,10 @@ describe('runs_selectors', () => {
10291030
});
10301031

10311032
describe('#getGroupedRunsTableHeaders', () => {
1032-
it('returns runs table headers grouped with other headers', () => {
1033-
const state = buildMockState({
1033+
let state: State;
1034+
1035+
beforeEach(() => {
1036+
state = buildMockState({
10341037
runs: buildRunsState(
10351038
{},
10361039
{
@@ -1081,38 +1084,70 @@ describe('runs_selectors', () => {
10811084
})
10821085
),
10831086
});
1087+
});
10841088

1089+
it('returns runs table headers grouped with other headers', () => {
10851090
expect(selectors.getGroupedRunsTableHeaders(state)).toEqual([
1086-
{
1091+
jasmine.objectContaining({
10871092
type: ColumnHeaderType.RUN,
10881093
name: 'run',
10891094
displayName: 'Run',
10901095
enabled: true,
1091-
},
1092-
{
1096+
}),
1097+
jasmine.objectContaining({
10931098
type: ColumnHeaderType.CUSTOM,
10941099
name: 'experimentAlias',
10951100
displayName: 'Experiment Alias',
10961101
enabled: true,
1097-
},
1098-
{
1102+
}),
1103+
jasmine.objectContaining({
10991104
type: ColumnHeaderType.HPARAM,
11001105
name: 'conv_layers',
11011106
displayName: 'Conv Layers',
11021107
enabled: true,
1103-
},
1104-
{
1108+
}),
1109+
jasmine.objectContaining({
11051110
type: ColumnHeaderType.HPARAM,
11061111
name: 'conv_kernel_size',
11071112
displayName: 'Conv Kernel Size',
11081113
enabled: true,
1109-
},
1110-
{
1114+
}),
1115+
jasmine.objectContaining({
11111116
type: ColumnHeaderType.COLOR,
11121117
name: 'color',
11131118
displayName: 'Color',
11141119
enabled: true,
1115-
},
1120+
}),
1121+
]);
1122+
});
1123+
1124+
it('sets the hparam column context options for the runs table', () => {
1125+
expect(selectors.getGroupedRunsTableHeaders(state)).toEqual([
1126+
jasmine.objectContaining({
1127+
type: ColumnHeaderType.RUN,
1128+
}),
1129+
jasmine.objectContaining({
1130+
type: ColumnHeaderType.CUSTOM,
1131+
}),
1132+
jasmine.objectContaining({
1133+
type: ColumnHeaderType.HPARAM,
1134+
name: 'conv_layers',
1135+
displayName: 'Conv Layers',
1136+
enabled: true,
1137+
removable: true,
1138+
hidable: false,
1139+
}),
1140+
jasmine.objectContaining({
1141+
type: ColumnHeaderType.HPARAM,
1142+
name: 'conv_kernel_size',
1143+
displayName: 'Conv Kernel Size',
1144+
enabled: true,
1145+
removable: true,
1146+
hidable: false,
1147+
}),
1148+
jasmine.objectContaining({
1149+
type: ColumnHeaderType.COLOR,
1150+
}),
11161151
]);
11171152
});
11181153
});

‎tensorboard/webapp/runs/views/runs_table/runs_data_table.ng.html

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
<tb-data-table
2929
[headers]="headers"
3030
[sortingInfo]="sortingInfo"
31-
[columnCustomizationEnabled]="true"
3231
[selectableColumns]="selectableColumns"
3332
[columnFilters]="columnFilters"
3433
[loading]="loading"
@@ -41,7 +40,6 @@
4140
<ng-container header>
4241
<ng-container *ngFor="let header of getHeaders()">
4342
<tb-data-table-header-cell
44-
*ngIf="header.enabled"
4543
[header]="header"
4644
[sortingInfo]="sortingInfo"
4745
[hparamsEnabled]="true"
@@ -69,7 +67,6 @@
6967
<tb-data-table-content-row [attr.data-id]="dataRow.id">
7068
<ng-container *ngFor="let header of getHeaders()">
7169
<tb-data-table-content-cell
72-
*ngIf="header.enabled"
7370
[header]="header"
7471
[datum]="dataRow[header.name]"
7572
[ngClass]="['table-column-' + header.name]"

‎tensorboard/webapp/runs/views/runs_table/runs_data_table.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
DiscreteFilter,
2929
IntervalFilter,
3030
ReorderColumnEvent,
31+
AddColumnEvent,
3132
} from '../../../widgets/data_table/types';
3233
@Component({
3334
selector: 'runs-data-table',
@@ -58,10 +59,7 @@ export class RunsDataTable {
5859
runId: string;
5960
newColor: string;
6061
}>();
61-
@Output() addColumn = new EventEmitter<{
62-
header: ColumnHeader;
63-
index?: number | undefined;
64-
}>();
62+
@Output() addColumn = new EventEmitter<AddColumnEvent>();
6563
@Output() removeColumn = new EventEmitter<ColumnHeader>();
6664
@Output() onSelectionDblClick = new EventEmitter<string>();
6765
@Output() addFilter = new EventEmitter<FilterAddedEvent>();

‎tensorboard/webapp/runs/views/runs_table/runs_data_table_test.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -140,21 +140,22 @@ describe('runs_data_table', () => {
140140
).toBeTruthy();
141141
});
142142

143-
it('projects enabled headers plus color and selected column', () => {
143+
it('projects headers plus color and selected column', () => {
144144
const fixture = createComponent({});
145145
const dataTable = fixture.debugElement.query(
146146
By.directive(DataTableComponent)
147147
);
148148
const headers = dataTable.queryAll(By.directive(HeaderCellComponent));
149149

150-
expect(headers.length).toBe(4);
150+
expect(headers.length).toBe(5);
151151
expect(headers[0].componentInstance.header.name).toEqual('selected');
152152
expect(headers[1].componentInstance.header.name).toEqual('run');
153-
expect(headers[2].componentInstance.header.name).toEqual('other_header');
154-
expect(headers[3].componentInstance.header.name).toEqual('color');
153+
expect(headers[2].componentInstance.header.name).toEqual('disabled_header');
154+
expect(headers[3].componentInstance.header.name).toEqual('other_header');
155+
expect(headers[4].componentInstance.header.name).toEqual('color');
155156
});
156157

157-
it('projects content for each enabled header, selected, and color column', () => {
158+
it('projects content for each header, selected, and color column', () => {
158159
const fixture = createComponent({
159160
data: [{id: 'runid', run: 'run name', color: 'red', other_header: 'foo'}],
160161
});
@@ -163,11 +164,12 @@ describe('runs_data_table', () => {
163164
);
164165
const cells = dataTable.queryAll(By.directive(ContentCellComponent));
165166

166-
expect(cells.length).toBe(4);
167+
expect(cells.length).toBe(5);
167168
expect(cells[0].componentInstance.header.name).toEqual('selected');
168169
expect(cells[1].componentInstance.header.name).toEqual('run');
169-
expect(cells[2].componentInstance.header.name).toEqual('other_header');
170-
expect(cells[3].componentInstance.header.name).toEqual('color');
170+
expect(cells[2].componentInstance.header.name).toEqual('disabled_header');
171+
expect(cells[3].componentInstance.header.name).toEqual('other_header');
172+
expect(cells[4].componentInstance.header.name).toEqual('color');
171173
});
172174

173175
describe('color column', () => {

0 commit comments

Comments
 (0)