Skip to content

Commit cf634de

Browse files
authored
HParams: Add new hparams selector factories (#6323)
## 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. Rather than further the practice of creating selectors with props ([which is deprecated](https://ngrx.io/guide/migration/v12)) I am choosing to create them using the recommended solution of a selector factory. Unfortunately selector factories don't play nicely with selectors dependent selectors WITH props. There are a handful of hacky solutions to this problem but I am opting to create new dependent selectors using the factory pattern. ## Technical description of changes I extracted the result functions from the existing selectors, then created new factories which call them with an additional closured parameter. ## Screenshots of UI changes None ## Detailed steps to verify changes work correctly (as executed by you) Tests should pass ## Alternate designs / implementations considered I could have just wrapped the existing selectors and then called them passing in the global state instead. I chose this approach instead with the hope of removing the old selectors in a later pr.
1 parent 42e85eb commit cf634de

File tree

3 files changed

+312
-57
lines changed

3 files changed

+312
-57
lines changed

tensorboard/webapp/hparams/_redux/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ tf_ts_library(
7474
":hparams_actions",
7575
":types",
7676
":utils",
77+
"//tensorboard/webapp:app_state",
7778
"//tensorboard/webapp/hparams:types",
7879
"//tensorboard/webapp/runs/actions",
7980
"@npm//@ngrx/store",

tensorboard/webapp/hparams/_redux/hparams_selectors.ts

Lines changed: 102 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -25,81 +25,126 @@ import {
2525
const getHparamsState =
2626
createFeatureSelector<HparamsState>(HPARAMS_FEATURE_KEY);
2727

28-
const getHparamsDefaultFiltersForExperiments = createSelector(
29-
getHparamsState,
30-
(
31-
state: HparamsState,
32-
experimentIds: string[]
33-
): Map<string, DiscreteFilter | IntervalFilter> => {
34-
const defaultFilterMaps: Array<
35-
Map<string, DiscreteFilter | IntervalFilter>
36-
> = [];
37-
38-
for (const experimentId of experimentIds) {
39-
if (!state.specs[experimentId]) {
40-
continue;
41-
}
42-
43-
defaultFilterMaps.push(state.specs[experimentId].hparam.defaultFilters);
28+
function getHparamsDefaultFiltersForExperimentsResultFunction(
29+
state: HparamsState,
30+
experimentIds: string[]
31+
): Map<string, DiscreteFilter | IntervalFilter> {
32+
const defaultFilterMaps: Array<Map<string, DiscreteFilter | IntervalFilter>> =
33+
[];
34+
35+
for (const experimentId of experimentIds) {
36+
if (!state.specs[experimentId]) {
37+
continue;
4438
}
4539

46-
return combineDefaultHparamFilters(defaultFilterMaps);
40+
defaultFilterMaps.push(state.specs[experimentId].hparam.defaultFilters);
4741
}
48-
);
4942

50-
export const getHparamFilterMap = createSelector(
51-
getHparamsDefaultFiltersForExperiments,
43+
return combineDefaultHparamFilters(defaultFilterMaps);
44+
}
45+
46+
const getHparamsDefaultFiltersForExperiments = createSelector(
5247
getHparamsState,
53-
(
54-
combinedDefaultfilterMap,
55-
hparamState,
56-
experimentIds: string[]
57-
): Map<string, IntervalFilter | DiscreteFilter> => {
58-
const id = getIdFromExperimentIds(experimentIds);
59-
const otherFilter = hparamState.filters[id];
60-
61-
return new Map([
62-
...combinedDefaultfilterMap,
63-
...(otherFilter?.hparams ?? []),
64-
]);
65-
}
48+
getHparamsDefaultFiltersForExperimentsResultFunction
6649
);
6750

68-
const getMetricsDefaultFiltersForExperiments = createSelector(
69-
getHparamsState,
70-
(
71-
state: HparamsState,
72-
experimentIds: string[]
73-
): Map<string, IntervalFilter> => {
74-
const defaultFilterMaps: Array<Map<string, IntervalFilter>> = [];
51+
function getHparamsDefaultFiltersForExperimentsFromExperimentIds(
52+
experimentIds: string[]
53+
) {
54+
return createSelector(getHparamsState, (state) =>
55+
getHparamsDefaultFiltersForExperimentsResultFunction(state, experimentIds)
56+
);
57+
}
58+
59+
function getHparamFilterMapResultFunction(
60+
hparamState: HparamsState,
61+
combinedDefaultfilterMap: Map<string, DiscreteFilter | IntervalFilter>,
62+
experimentIds: string[]
63+
): Map<string, IntervalFilter | DiscreteFilter> {
64+
const id = getIdFromExperimentIds(experimentIds);
65+
const otherFilter = hparamState.filters[id];
66+
67+
return new Map([
68+
...combinedDefaultfilterMap,
69+
...(otherFilter?.hparams ?? []),
70+
]);
71+
}
7572

76-
for (const experimentId of experimentIds) {
77-
if (!state.specs[experimentId]) {
78-
continue;
79-
}
73+
export const getHparamFilterMap = createSelector(
74+
getHparamsState,
75+
getHparamsDefaultFiltersForExperiments,
76+
getHparamFilterMapResultFunction
77+
);
8078

81-
defaultFilterMaps.push(state.specs[experimentId].metric.defaultFilters);
79+
export function getHparamFilterMapFromExperimentIds(experimentIds: string[]) {
80+
return createSelector(
81+
getHparamsState,
82+
getHparamsDefaultFiltersForExperimentsFromExperimentIds(experimentIds),
83+
(hparamState, combinedDefaultFilterMap) =>
84+
getHparamFilterMapResultFunction(
85+
hparamState,
86+
combinedDefaultFilterMap,
87+
experimentIds
88+
)
89+
);
90+
}
91+
92+
function getMetricsDefaultFiltersForExperimentsResultFunction(
93+
state: HparamsState,
94+
experimentIds: string[]
95+
): Map<string, IntervalFilter> {
96+
const defaultFilterMaps: Array<Map<string, IntervalFilter>> = [];
97+
98+
for (const experimentId of experimentIds) {
99+
if (!state.specs[experimentId]) {
100+
continue;
82101
}
83102

84-
return combineDefaultMetricFilters(defaultFilterMaps);
103+
defaultFilterMaps.push(state.specs[experimentId].metric.defaultFilters);
85104
}
105+
106+
return combineDefaultMetricFilters(defaultFilterMaps);
107+
}
108+
109+
const getMetricsDefaultFiltersForExperiments = createSelector(
110+
getHparamsState,
111+
getMetricsDefaultFiltersForExperimentsResultFunction
86112
);
87113

114+
function getMetricsDefaultFiltersForExperimentsFromExperimentIds(
115+
experimentIds: string[]
116+
) {
117+
return createSelector(getHparamsState, (state) =>
118+
getMetricsDefaultFiltersForExperimentsResultFunction(state, experimentIds)
119+
);
120+
}
121+
122+
function getMetricFilterMapResultFunction(
123+
hparamState: HparamsState,
124+
defaultFilterMap: Map<string, IntervalFilter>,
125+
experimentIds: string[]
126+
): Map<string, IntervalFilter> {
127+
const id = getIdFromExperimentIds(experimentIds);
128+
const otherFilter = hparamState.filters[id];
129+
130+
return new Map([...defaultFilterMap, ...(otherFilter?.metrics ?? [])]);
131+
}
132+
88133
export const getMetricFilterMap = createSelector(
89-
getMetricsDefaultFiltersForExperiments,
90134
getHparamsState,
91-
(
92-
defaultfilterMap,
93-
hparamState,
94-
experimentIds: string[]
95-
): Map<string, IntervalFilter> => {
96-
const id = getIdFromExperimentIds(experimentIds);
97-
const otherFilter = hparamState.filters[id];
98-
99-
return new Map([...defaultfilterMap, ...(otherFilter?.metrics ?? [])]);
100-
}
135+
getMetricsDefaultFiltersForExperiments,
136+
getMetricFilterMapResultFunction
101137
);
102138

139+
export function getMetricFilterMapFromExperimentIds(experimentIds: string[]) {
140+
return createSelector(
141+
getHparamsState,
142+
getMetricsDefaultFiltersForExperimentsFromExperimentIds(experimentIds),
143+
(state, defaultFilterMap) =>
144+
getMetricFilterMapResultFunction(state, defaultFilterMap, experimentIds)
145+
);
146+
}
147+
103148
/**
104149
* Returns Observable that emits hparams and metrics specs of experiments.
105150
*/

0 commit comments

Comments
 (0)