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

Makes runs table use hparam store for hparam columns #6736

Merged
merged 19 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions tensorboard/webapp/hparams/_redux/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ tf_ts_library(
":types",
":utils",
"//tensorboard/webapp/hparams:_types",
"//tensorboard/webapp/persistent_settings",
"//tensorboard/webapp/runs/actions",
"//tensorboard/webapp/widgets/data_table:types",
"//tensorboard/webapp/widgets/data_table:utils",
Expand Down Expand Up @@ -168,6 +169,7 @@ tf_ts_library(
"//tensorboard/webapp/app_routing/actions",
"//tensorboard/webapp/core/actions",
"//tensorboard/webapp/hparams:types",
"//tensorboard/webapp/persistent_settings",
"//tensorboard/webapp/runs/actions",
"//tensorboard/webapp/runs/data_source:testing",
"//tensorboard/webapp/runs/store:testing",
Expand Down
13 changes: 12 additions & 1 deletion tensorboard/webapp/hparams/_redux/hparams_reducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/
import {Action, ActionReducer, createReducer, on} from '@ngrx/store';
import {Side} from '../../widgets/data_table/types';
import {DataTableUtils} from '../../widgets/data_table/utils';
import {persistentSettingsLoaded} from '../../persistent_settings';
import {Side} from '../../widgets/data_table/types';
import * as actions from './hparams_actions';
import {HparamsState} from './types';

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

const reducer: ActionReducer<HparamsState, Action> = createReducer(
initialState,
on(persistentSettingsLoaded, (state, {partialSettings}) => {
const {dashboardDisplayedHparamColumns: storedColumns} = partialSettings;
if (storedColumns) {
return {
...state,
dashboardDisplayedHparamColumns: storedColumns,
};
}
return state;
}),
on(actions.hparamsFetchSessionGroupsSucceeded, (state, action) => {
const nextDashboardSpecs = action.hparamsAndMetricsSpecs;
const nextDashboardSessionGroups = action.sessionGroups;
Expand Down
73 changes: 73 additions & 0 deletions tensorboard/webapp/hparams/_redux/hparams_reducers_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,81 @@ import {reducers} from './hparams_reducers';
import {buildHparamSpec, buildHparamsState, buildMetricSpec} from './testing';
import {ColumnHeaderType, Side} from '../../widgets/data_table/types';
import {DataTableUtils} from '../../widgets/data_table/utils';
import {persistentSettingsLoaded} from '../../persistent_settings';

describe('hparams/_redux/hparams_reducers_test', () => {
describe('#persistentSettingsLoaded', () => {
it('loads dashboardDisplayedHparamColumns from the persistent settings storage', () => {
const state = buildHparamsState({
dashboardDisplayedHparamColumns: [],
});
const state2 = reducers(
state,
persistentSettingsLoaded({
partialSettings: {
dashboardDisplayedHparamColumns: [
{
type: ColumnHeaderType.HPARAM,
name: 'conv_layers',
displayName: 'Conv Layers',
enabled: true,
},
{
type: ColumnHeaderType.HPARAM,
name: 'conv_kernel_size',
displayName: 'Conv Kernel Size',
enabled: true,
},
],
},
})
);

expect(state2.dashboardDisplayedHparamColumns).toEqual([
{
type: ColumnHeaderType.HPARAM,
name: 'conv_layers',
displayName: 'Conv Layers',
enabled: true,
},
{
type: ColumnHeaderType.HPARAM,
name: 'conv_kernel_size',
displayName: 'Conv Kernel Size',
enabled: true,
},
]);
});

it('does nothing if persistent settings does not contain dashboardDisplayedHparamColumns', () => {
const state = buildHparamsState({
dashboardDisplayedHparamColumns: [
{
type: ColumnHeaderType.HPARAM,
name: 'conv_layers',
displayName: 'Conv Layers',
enabled: true,
},
],
});
const state2 = reducers(
state,
persistentSettingsLoaded({
partialSettings: {},
})
);

expect(state2.dashboardDisplayedHparamColumns).toEqual([
{
type: ColumnHeaderType.HPARAM,
name: 'conv_layers',
displayName: 'Conv Layers',
enabled: true,
},
]);
});
});

describe('hparamsFetchSessionGroupsSucceeded', () => {
it('saves action.hparamsAndMetricsSpecs as dashboardSpecs', () => {
const state = buildHparamsState({
Expand Down
1 change: 1 addition & 0 deletions tensorboard/webapp/runs/store/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ tf_ts_library(
":testing",
":types",
":utils",
"//tensorboard/webapp:app_state",
"//tensorboard/webapp/app_routing:testing",
"//tensorboard/webapp/app_routing:types",
"//tensorboard/webapp/app_routing/actions",
Expand Down
14 changes: 12 additions & 2 deletions tensorboard/webapp/runs/store/runs_selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,16 @@ export const getRunsTableSortingInfo = createSelector(
export const getGroupedRunsTableHeaders = createSelector(
getRunsTableHeaders,
getDashboardDisplayedHparamColumns,
(runsTableHeaders, hparamColumns) =>
DataTableUtils.groupColumns([...runsTableHeaders, ...hparamColumns])
(runsTableHeaders, hparamColumns) => {
// Override hparam options to match runs table requirements.
const columns = [...runsTableHeaders, ...hparamColumns].map((column) => {
const newColumn = {...column};
if (column.type === 'HPARAM') {
newColumn.removable = true;
newColumn.hidable = false;
}
return newColumn;
});
return DataTableUtils.groupColumns(columns);
}
);
59 changes: 47 additions & 12 deletions tensorboard/webapp/runs/store/runs_selectors_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
buildHparamSpec,
} from '../../hparams/testing';
import {buildMockState} from '../../testing/utils';
import {State} from '../../app_state';
import {DataLoadState} from '../../types/data';
import {ColumnHeaderType, SortingOrder} from '../../widgets/data_table/types';
import {GroupByKey} from '../types';
Expand Down Expand Up @@ -1029,8 +1030,10 @@ describe('runs_selectors', () => {
});

describe('#getGroupedRunsTableHeaders', () => {
it('returns runs table headers grouped with other headers', () => {
const state = buildMockState({
let state: State;

beforeEach(() => {
state = buildMockState({
runs: buildRunsState(
{},
{
Expand Down Expand Up @@ -1081,38 +1084,70 @@ describe('runs_selectors', () => {
})
),
});
});

it('returns runs table headers grouped with other headers', () => {
expect(selectors.getGroupedRunsTableHeaders(state)).toEqual([
{
jasmine.objectContaining({
type: ColumnHeaderType.RUN,
name: 'run',
displayName: 'Run',
enabled: true,
},
{
}),
jasmine.objectContaining({
type: ColumnHeaderType.CUSTOM,
name: 'experimentAlias',
displayName: 'Experiment Alias',
enabled: true,
},
{
}),
jasmine.objectContaining({
type: ColumnHeaderType.HPARAM,
name: 'conv_layers',
displayName: 'Conv Layers',
enabled: true,
},
{
}),
jasmine.objectContaining({
type: ColumnHeaderType.HPARAM,
name: 'conv_kernel_size',
displayName: 'Conv Kernel Size',
enabled: true,
},
{
}),
jasmine.objectContaining({
type: ColumnHeaderType.COLOR,
name: 'color',
displayName: 'Color',
enabled: true,
},
}),
]);
});

it('sets the hparam column context options for the runs table', () => {
expect(selectors.getGroupedRunsTableHeaders(state)).toEqual([
jasmine.objectContaining({
type: ColumnHeaderType.RUN,
}),
jasmine.objectContaining({
type: ColumnHeaderType.CUSTOM,
}),
jasmine.objectContaining({
type: ColumnHeaderType.HPARAM,
name: 'conv_layers',
displayName: 'Conv Layers',
enabled: true,
removable: true,
hidable: false,
}),
jasmine.objectContaining({
type: ColumnHeaderType.HPARAM,
name: 'conv_kernel_size',
displayName: 'Conv Kernel Size',
enabled: true,
removable: true,
hidable: false,
}),
jasmine.objectContaining({
type: ColumnHeaderType.COLOR,
}),
]);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
<tb-data-table
[headers]="headers"
[sortingInfo]="sortingInfo"
[columnCustomizationEnabled]="true"
[selectableColumns]="selectableColumns"
[columnFilters]="columnFilters"
[loading]="loading"
Expand All @@ -41,7 +40,6 @@
<ng-container header>
<ng-container *ngFor="let header of getHeaders()">
<tb-data-table-header-cell
*ngIf="header.enabled"
[header]="header"
[sortingInfo]="sortingInfo"
[hparamsEnabled]="true"
Expand Down Expand Up @@ -69,7 +67,6 @@
<tb-data-table-content-row [attr.data-id]="dataRow.id">
<ng-container *ngFor="let header of getHeaders()">
<tb-data-table-content-cell
*ngIf="header.enabled"
[header]="header"
[datum]="dataRow[header.name]"
[ngClass]="['table-column-' + header.name]"
Expand Down
6 changes: 2 additions & 4 deletions tensorboard/webapp/runs/views/runs_table/runs_data_table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
DiscreteFilter,
IntervalFilter,
ReorderColumnEvent,
AddColumnEvent,
} from '../../../widgets/data_table/types';
@Component({
selector: 'runs-data-table',
Expand Down Expand Up @@ -58,10 +59,7 @@ export class RunsDataTable {
runId: string;
newColor: string;
}>();
@Output() addColumn = new EventEmitter<{
header: ColumnHeader;
index?: number | undefined;
}>();
@Output() addColumn = new EventEmitter<AddColumnEvent>();
@Output() removeColumn = new EventEmitter<ColumnHeader>();
@Output() onSelectionDblClick = new EventEmitter<string>();
@Output() addFilter = new EventEmitter<FilterAddedEvent>();
Expand Down
18 changes: 10 additions & 8 deletions tensorboard/webapp/runs/views/runs_table/runs_data_table_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,21 +140,22 @@ describe('runs_data_table', () => {
).toBeTruthy();
});

it('projects enabled headers plus color and selected column', () => {
it('projects headers plus color and selected column', () => {
const fixture = createComponent({});
const dataTable = fixture.debugElement.query(
By.directive(DataTableComponent)
);
const headers = dataTable.queryAll(By.directive(HeaderCellComponent));

expect(headers.length).toBe(4);
expect(headers.length).toBe(5);
expect(headers[0].componentInstance.header.name).toEqual('selected');
expect(headers[1].componentInstance.header.name).toEqual('run');
expect(headers[2].componentInstance.header.name).toEqual('other_header');
expect(headers[3].componentInstance.header.name).toEqual('color');
expect(headers[2].componentInstance.header.name).toEqual('disabled_header');
expect(headers[3].componentInstance.header.name).toEqual('other_header');
expect(headers[4].componentInstance.header.name).toEqual('color');
});

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

expect(cells.length).toBe(4);
expect(cells.length).toBe(5);
expect(cells[0].componentInstance.header.name).toEqual('selected');
expect(cells[1].componentInstance.header.name).toEqual('run');
expect(cells[2].componentInstance.header.name).toEqual('other_header');
expect(cells[3].componentInstance.header.name).toEqual('color');
expect(cells[2].componentInstance.header.name).toEqual('disabled_header');
expect(cells[3].componentInstance.header.name).toEqual('other_header');
expect(cells[4].componentInstance.header.name).toEqual('color');
});

describe('color column', () => {
Expand Down
Loading
Loading