Skip to content

Commit 91a637e

Browse files
rileyajonesrileyajones
authored
Hparams: refactor arguments provided to getDashboardRuns (#6555)
## Motivation for features / changes As per a discussion around #6544 we determined that the dashboard should not be requesting runs per experiment but should instead be getting all runs for all experiments shown. This greatly simplifies a lot of the logic used to retrieve runs and run related data.
1 parent d190899 commit 91a637e

File tree

12 files changed

+269
-232
lines changed

12 files changed

+269
-232
lines changed

‎tensorboard/webapp/experiments/store/BUILD‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ tf_ts_library(
2626
],
2727
deps = [
2828
":types",
29+
"//tensorboard/webapp/app_routing/store",
2930
"//tensorboard/webapp/experiments:types",
3031
"//tensorboard/webapp/types",
3132
"@npm//@ngrx/store",
@@ -65,6 +66,10 @@ tf_ts_library(
6566
":selectors",
6667
":testing",
6768
":types",
69+
"//tensorboard/webapp:app_state",
70+
"//tensorboard/webapp/app_routing:types",
71+
"//tensorboard/webapp/app_routing/store:testing",
72+
"//tensorboard/webapp/testing:utils",
6873
"//tensorboard/webapp/types",
6974
"@npm//@types/jasmine",
7075
],

‎tensorboard/webapp/experiments/store/experiments_selectors.ts‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
ExperimentsState,
2020
EXPERIMENTS_FEATURE_KEY,
2121
} from './experiments_types';
22+
import {getExperimentIdsFromRoute} from '../../app_routing/store/app_routing_selectors';
2223

2324
const getExperimentsState = createFeatureSelector<ExperimentsState>(
2425
EXPERIMENTS_FEATURE_KEY
@@ -46,18 +47,20 @@ export const getExperiment = createSelector(
4647
);
4748

4849
/**
49-
* Returns Observable that emits an object mapping the provided
50-
* experiment ids to experiment names.
50+
* Returns the names of all experiments present on the current dashboard.
5151
*/
52-
export const getExperimentNames = (experimentIds: string[]) =>
53-
createSelector(
54-
getDataState,
55-
(state: ExperimentsDataState): Record<string, string> =>
56-
experimentIds
57-
.map((experimentId) => state.experimentMap[experimentId])
58-
.filter(Boolean)
59-
.reduce((map, experiment) => {
60-
map[experiment.id] = experiment.name;
61-
return map;
62-
}, {} as Record<string, string>)
63-
);
52+
export const getDashboardExperimentNames = createSelector(
53+
getDataState,
54+
getExperimentIdsFromRoute,
55+
(
56+
state: ExperimentsDataState,
57+
experimentIds: string[] | null
58+
): Record<string, string> =>
59+
(experimentIds ?? [])
60+
.map((experimentId) => state.experimentMap[experimentId])
61+
.filter(Boolean)
62+
.reduce((map, experiment) => {
63+
map[experiment.id] = experiment.name;
64+
return map;
65+
}, {} as Record<string, string>)
66+
);

‎tensorboard/webapp/experiments/store/experiments_selectors_test.ts‎

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,14 @@ See the License for the specific language governing permissions and
1313
limitations under the License.
1414
==============================================================================*/
1515
import * as selectors from './experiments_selectors';
16-
import {State} from './experiments_types';
16+
import {State} from '../../app_state';
1717
import {buildExperiment, buildStateFromExperimentsState} from './testing';
18+
import {buildMockState} from '../../testing/utils';
19+
import {
20+
buildAppRoutingState,
21+
buildStateFromAppRoutingState,
22+
} from '../../app_routing/store/testing';
23+
import {RouteKind} from '../../app_routing/types';
1824

1925
describe('experiments selectors', () => {
2026
describe('#getExperiment', () => {
@@ -49,32 +55,52 @@ describe('experiments selectors', () => {
4955
});
5056
});
5157

52-
describe('#getExperimentNames', () => {
58+
describe('#getDashboardExperimentNames', () => {
5359
let state: State;
5460

5561
beforeEach(() => {
5662
const foo = buildExperiment({id: 'foo', name: 'foo name'});
5763
const bar = buildExperiment({id: 'bar', name: 'bar name'});
5864

59-
state = buildStateFromExperimentsState({
60-
data: {
61-
experimentMap: {foo, bar},
62-
},
65+
state = buildMockState({
66+
...buildStateFromAppRoutingState(
67+
buildAppRoutingState({
68+
activeRoute: {
69+
routeKind: RouteKind.COMPARE_EXPERIMENT,
70+
params: {
71+
experimentIds: 'exp1:foo,exp2:bar,exp3:baz',
72+
},
73+
},
74+
})
75+
),
76+
...buildStateFromExperimentsState({
77+
data: {
78+
experimentMap: {foo, bar},
79+
},
80+
}),
6381
});
6482
});
6583

6684
it('translates experiment ids to experiment names', () => {
67-
expect(
68-
selectors.getExperimentNames(['foo', 'bar', 'baz'])(state)
69-
).toEqual({
85+
expect(selectors.getDashboardExperimentNames(state)).toEqual({
7086
foo: 'foo name',
7187
bar: 'bar name',
7288
});
7389
});
7490

7591
it('returns an empty object when no experiments are provided', () => {
76-
expect(selectors.getExperimentNames([])(state)).toEqual({});
77-
expect(selectors.getExperimentNames(['abc', '123'])(state)).toEqual({});
92+
state = {
93+
...state,
94+
...buildStateFromAppRoutingState(
95+
buildAppRoutingState({
96+
activeRoute: {
97+
routeKind: RouteKind.EXPERIMENTS,
98+
params: {},
99+
},
100+
})
101+
),
102+
};
103+
expect(selectors.getDashboardExperimentNames(state)).toEqual({});
78104
});
79105
});
80106
});

‎tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ import {
100100
HeaderToggleInfo,
101101
XAxisType,
102102
} from '../../types';
103-
import {getFilteredRenderableRunsIdsFromRoute} from '../main_view/common_selectors';
103+
import {getFilteredRenderableRunsIds} from '../main_view/common_selectors';
104104
import {CardRenderer} from '../metrics_view_types';
105105
import {getTagDisplayName} from '../utils';
106106
import {DataDownloadDialogContainer} from './data_download_dialog_container';
@@ -499,7 +499,7 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy {
499499
combineLatestWith(
500500
this.store.select(getCurrentRouteRunSelection),
501501
this.store.select(getEnableHparamsInTimeSeries),
502-
this.store.select(getFilteredRenderableRunsIdsFromRoute),
502+
this.store.select(getFilteredRenderableRunsIds),
503503
this.store.select(getRunColorMap),
504504
this.store.select(getMetricsScalarSmoothing)
505505
),

‎tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ describe('scalar card', () => {
485485
new Map([['run1', true]])
486486
);
487487
store.overrideSelector(
488-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
488+
commonSelectors.getFilteredRenderableRunsIds,
489489
new Set(['run1'])
490490
);
491491
store.overrideSelector(selectors.getMetricsXAxisType, XAxisType.STEP);
@@ -764,7 +764,7 @@ describe('scalar card', () => {
764764
new Map([['run1', true]])
765765
);
766766
store.overrideSelector(
767-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
767+
commonSelectors.getFilteredRenderableRunsIds,
768768
new Set(['run1'])
769769
);
770770
});
@@ -2574,7 +2574,7 @@ describe('scalar card', () => {
25742574
])
25752575
);
25762576
store.overrideSelector(
2577-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
2577+
commonSelectors.getFilteredRenderableRunsIds,
25782578
new Set(['run1', 'run2'])
25792579
);
25802580
store.overrideSelector(getCardStateMap, {
@@ -2996,7 +2996,7 @@ describe('scalar card', () => {
29962996
])
29972997
);
29982998
store.overrideSelector(
2999-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
2999+
commonSelectors.getFilteredRenderableRunsIds,
30003000
new Set(['run1', 'run2'])
30013001
);
30023002

@@ -3066,7 +3066,7 @@ describe('scalar card', () => {
30663066
])
30673067
);
30683068
store.overrideSelector(
3069-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3069+
commonSelectors.getFilteredRenderableRunsIds,
30703070
new Set(['run1', 'run2'])
30713071
);
30723072

@@ -3145,7 +3145,7 @@ describe('scalar card', () => {
31453145
new Map([['run1', true]])
31463146
);
31473147
store.overrideSelector(
3148-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3148+
commonSelectors.getFilteredRenderableRunsIds,
31493149
new Set(['run1'])
31503150
);
31513151

@@ -3211,7 +3211,7 @@ describe('scalar card', () => {
32113211
new Map([['run1', true]])
32123212
);
32133213
store.overrideSelector(
3214-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3214+
commonSelectors.getFilteredRenderableRunsIds,
32153215
new Set(['run1'])
32163216
);
32173217

@@ -3278,7 +3278,7 @@ describe('scalar card', () => {
32783278
])
32793279
);
32803280
store.overrideSelector(
3281-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3281+
commonSelectors.getFilteredRenderableRunsIds,
32823282
new Set(['run1', 'run2'])
32833283
);
32843284

@@ -3330,7 +3330,7 @@ describe('scalar card', () => {
33303330
])
33313331
);
33323332
store.overrideSelector(
3333-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3333+
commonSelectors.getFilteredRenderableRunsIds,
33343334
new Set(['run1', 'run2'])
33353335
);
33363336

@@ -3382,7 +3382,7 @@ describe('scalar card', () => {
33823382
])
33833383
);
33843384
store.overrideSelector(
3385-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3385+
commonSelectors.getFilteredRenderableRunsIds,
33863386
new Set(['run1', 'run2'])
33873387
);
33883388

@@ -3432,7 +3432,7 @@ describe('scalar card', () => {
34323432
])
34333433
);
34343434
store.overrideSelector(
3435-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3435+
commonSelectors.getFilteredRenderableRunsIds,
34363436
new Set(['run1', 'run2'])
34373437
);
34383438
store.overrideSelector(getMetricsLinkedTimeSelection, {
@@ -3481,7 +3481,7 @@ describe('scalar card', () => {
34813481
])
34823482
);
34833483
store.overrideSelector(
3484-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3484+
commonSelectors.getFilteredRenderableRunsIds,
34853485
new Set(['run1', 'run2'])
34863486
);
34873487
store.overrideSelector(selectors.getExperimentIdToExperimentAliasMap, {
@@ -3539,7 +3539,7 @@ describe('scalar card', () => {
35393539
new Map([['run1', true]])
35403540
);
35413541
store.overrideSelector(
3542-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3542+
commonSelectors.getFilteredRenderableRunsIds,
35433543
new Set(['run1'])
35443544
);
35453545
store.overrideSelector(getMetricsLinkedTimeSelection, {
@@ -3586,7 +3586,7 @@ describe('scalar card', () => {
35863586
])
35873587
);
35883588
store.overrideSelector(
3589-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3589+
commonSelectors.getFilteredRenderableRunsIds,
35903590
new Set(['run1', 'run2', 'run3'])
35913591
);
35923592

@@ -3635,7 +3635,7 @@ describe('scalar card', () => {
36353635
])
36363636
);
36373637
store.overrideSelector(
3638-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3638+
commonSelectors.getFilteredRenderableRunsIds,
36393639
new Set(['run1', 'run2', 'run3'])
36403640
);
36413641

@@ -3692,7 +3692,7 @@ describe('scalar card', () => {
36923692
])
36933693
);
36943694
store.overrideSelector(
3695-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3695+
commonSelectors.getFilteredRenderableRunsIds,
36963696
new Set(['run1', 'run2', 'run3', 'run4', 'run5', 'run6', 'run7'])
36973697
);
36983698

@@ -3754,7 +3754,7 @@ describe('scalar card', () => {
37543754
);
37553755

37563756
store.overrideSelector(
3757-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3757+
commonSelectors.getFilteredRenderableRunsIds,
37583758
new Set(['run1', 'run2', 'run3', 'run4', 'run5', 'run6', 'run7'])
37593759
);
37603760

@@ -3840,7 +3840,7 @@ describe('scalar card', () => {
38403840
);
38413841

38423842
store.overrideSelector(
3843-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3843+
commonSelectors.getFilteredRenderableRunsIds,
38443844
new Set(['run1'])
38453845
);
38463846

@@ -3952,7 +3952,7 @@ describe('scalar card', () => {
39523952
])
39533953
);
39543954
store.overrideSelector(
3955-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3955+
commonSelectors.getFilteredRenderableRunsIds,
39563956
new Set(['run1', 'run2', 'run3', 'run4'])
39573957
);
39583958

@@ -3981,7 +3981,7 @@ describe('scalar card', () => {
39813981
])
39823982
);
39833983
store.overrideSelector(
3984-
commonSelectors.getFilteredRenderableRunsIdsFromRoute,
3984+
commonSelectors.getFilteredRenderableRunsIds,
39853985
new Set(['run1', 'run2'])
39863986
);
39873987
const fixture = createComponent('card1');

0 commit comments

Comments
 (0)