Skip to content

HParams: Update the runs table to read columns from redux state #6404

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
May 25, 2023
Merged

HParams: Update the runs table to read columns from redux state #6404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
62f671a
update the runs table to read columns from redux state
rileyajones May 24, 2023
48bbc0e
address pr comments
rileyajones May 25, 2023
13e3ba5
remove unused dependency
rileyajones May 25, 2023
5a53cc2
reformat build file
rileyajones May 25, 2023
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
1 change: 1 addition & 0 deletions tensorboard/webapp/runs/actions/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
17 changes: 17 additions & 0 deletions tensorboard/webapp/runs/actions/runs_actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}>()
);
5 changes: 5 additions & 0 deletions tensorboard/webapp/runs/data_source/runs_data_source_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,8 @@ export abstract class RunsDataSource {
experimentId: string
): Observable<HparamsAndMetadata>;
}

export type RunToHParamValues = Record<
string,
Map<string, HparamValue['value']>
>;
5 changes: 5 additions & 0 deletions tensorboard/webapp/runs/store/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
Expand All @@ -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",
],
)
Expand All @@ -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",
],
)

Expand All @@ -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",
],
)

Expand Down Expand Up @@ -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",
],
)
39 changes: 38 additions & 1 deletion tensorboard/webapp/runs/store/runs_reducers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -309,14 +311,30 @@ const initialSort: RunsUiNamespacedState['sort'] = {
direction: SortDirection.UNSET,
};
const {initialState: uiInitialState, reducers: uiNamespaceContextedReducers} =
createNamespaceContextedState(
createNamespaceContextedState<
RunsUiNamespacedState,
RunsUiNonNamespacedState
>(
{
paginationOption: {
pageIndex: 0,
pageSize: 10,
},
sort: initialSort,
selectionState: new Map<string, boolean>(),
runsTableHeaders: [
{
type: ColumnHeaderType.RUN,
name: 'run',
displayName: 'Run',
enabled: true,
},
],
sortingInfo: {
header: ColumnHeaderType.RUN,
name: 'run',
order: SortingOrder.DESCENDING,
},
},
{}
);
Expand Down Expand Up @@ -407,6 +425,25 @@ const uiReducer: ActionReducer<RunsUiState, Action> = 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,
};
})
);

Expand Down
102 changes: 101 additions & 1 deletion tensorboard/webapp/runs/store/runs_reducers_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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('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,
});
});
});
});
21 changes: 21 additions & 0 deletions tensorboard/webapp/runs/store/runs_selectors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<RunsState>(RUNS_FEATURE_KEY);

Expand Down Expand Up @@ -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;
}
);
63 changes: 63 additions & 0 deletions tensorboard/webapp/runs/store/runs_selectors_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -584,4 +585,66 @@ describe('runs_selectors', () => {
expect(selectors.getColorGroupRegexString(state)).toEqual('foo(\\d+)');
});
});

describe('#getRunsTableHeaders', () => {
Copy link
Contributor

@JamesHollyer JamesHollyer May 24, 2023

Choose a reason for hiding this comment

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

I would actually be fine without tests for simple return selectors with no logic. They are written so leave them. Maybe we can discuss this with the FE team sometime.

Copy link
Contributor Author

@rileyajones rileyajones May 25, 2023

Choose a reason for hiding this comment

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

That does seem like a good topic for an FE meeting. It feels like a bit of a waste of time to write them but it's been the convention to do so

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,
});
});
});
});
3 changes: 3 additions & 0 deletions tensorboard/webapp/runs/store/runs_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -79,6 +80,8 @@ export interface RunsUiNamespacedState {
* Indicates whether the run is selected.
*/
selectionState: Map<RunId, boolean>;
runsTableHeaders: ColumnHeader[];
sortingInfo: SortingInfo;
}

export interface RunsUiNonNamespacedState {}
Expand Down
Loading