Skip to content

Commit a0bddaf

Browse files
bmd3kbmd3k
authored
Hparams: Limit number of Hparams retrieved by default. (#6777)
Limit the number of hparams retrieved by default to 1000 for display in Time Series. Print retrieval information: # of columns loaded and whether there could be more. This is to address performance issues with loading 1000s of hparams from the backend and trying to render them all. In a future PR we will add ability to "Load All" if user wants to nonetheless load them all. Screenshot (with default set to 2 instead of 1000): ![image](https://github.com/tensorflow/tensorboard/assets/17152369/b0f2c4e6-bda0-4ed7-bead-9f1ef6eaaca7)
1 parent 6f5be4b commit a0bddaf

26 files changed

+242
-42
lines changed

‎tensorboard/webapp/hparams/_redux/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ tf_ng_module(
111111
deps = [
112112
":hparams_actions",
113113
":hparams_data_source",
114+
":hparams_selectors",
114115
":types",
115116
"//tensorboard/webapp:app_state",
116117
"//tensorboard/webapp/app_routing:types",

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,15 @@ export class HparamsDataSource {
8787
return experimentIds.map((eid, index) => `${index}:${eid}`).join(',');
8888
}
8989

90-
fetchExperimentInfo(experimentIds: string[]): Observable<HparamSpec[]> {
90+
fetchExperimentInfo(
91+
experimentIds: string[],
92+
hparamsLimit: number
93+
): Observable<HparamSpec[]> {
9194
const formattedExperimentIds = this.formatExperimentIds(experimentIds);
9295

9396
const experimentRequest: BackendHparamsExperimentRequest = {
9497
experimentName: formattedExperimentIds,
98+
hparamsLimit,
9599
// The hparams feature generates its own metric data and does not require
96100
// the backend to calculate it.
97101
includeMetrics: false,
@@ -132,7 +136,7 @@ export class HparamsDataSource {
132136
const colParams: BackendListSessionGroupRequest['colParams'] = [];
133137

134138
for (const hparamSpec of hparamSpecs) {
135-
colParams.push({hparam: hparamSpec.name});
139+
colParams.push({hparam: hparamSpec.name, includeInResult: true});
136140
}
137141

138142
const listSessionRequestParams: BackendListSessionGroupRequest = {

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

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
BackendListSessionGroupResponse,
2929
RunStatus,
3030
} from '../types';
31+
import {buildHparamSpec} from './testing';
3132

3233
describe('HparamsDataSource Test', () => {
3334
let httpMock: HttpTestingController;
@@ -46,7 +47,7 @@ describe('HparamsDataSource Test', () => {
4647
describe('fetchExperimentInfo', () => {
4748
it('uses /experiment when a single experiment id is provided', () => {
4849
const returnValue = jasmine.createSpy();
49-
dataSource.fetchExperimentInfo(['eid']).subscribe(returnValue);
50+
dataSource.fetchExperimentInfo(['eid'], 0).subscribe(returnValue);
5051
httpMock
5152
.expectOne('/experiment/eid/data/plugin/hparams/experiment')
5253
.flush(createHparamsExperimentResponse());
@@ -55,7 +56,9 @@ describe('HparamsDataSource Test', () => {
5556

5657
it('uses /compare when a multiple experiment ids are provided', () => {
5758
const returnValue = jasmine.createSpy();
58-
dataSource.fetchExperimentInfo(['eid1', 'eid2']).subscribe(returnValue);
59+
dataSource
60+
.fetchExperimentInfo(['eid1', 'eid2'], 0)
61+
.subscribe(returnValue);
5962
httpMock
6063
.expectOne('/compare/0:eid1,1:eid2/data/plugin/hparams/experiment')
6164
.flush(createHparamsExperimentResponse());
@@ -64,7 +67,7 @@ describe('HparamsDataSource Test', () => {
6467

6568
it('maps interval and discrete domains to domain', () => {
6669
const returnValue = jasmine.createSpy();
67-
dataSource.fetchExperimentInfo(['eid']).subscribe(returnValue);
70+
dataSource.fetchExperimentInfo(['eid'], 0).subscribe(returnValue);
6871
httpMock
6972
.expectOne('/experiment/eid/data/plugin/hparams/experiment')
7073
.flush(createHparamsExperimentResponse());
@@ -95,7 +98,7 @@ describe('HparamsDataSource Test', () => {
9598

9699
it('treats missing domains as discrete domains', () => {
97100
const returnValue = jasmine.createSpy();
98-
dataSource.fetchExperimentInfo(['eid']).subscribe(returnValue);
101+
dataSource.fetchExperimentInfo(['eid'], 0).subscribe(returnValue);
99102
httpMock
100103
.expectOne('/experiment/eid/data/plugin/hparams/experiment')
101104
.flush(createHparamsExperimentNoDomainResponse());
@@ -122,6 +125,15 @@ describe('HparamsDataSource Test', () => {
122125
},
123126
]);
124127
});
128+
129+
it('sets hparamsLimit and includeMetrics', () => {
130+
dataSource.fetchExperimentInfo(['eid'], 100).subscribe();
131+
const actual = httpMock.expectOne(
132+
'/experiment/eid/data/plugin/hparams/experiment'
133+
);
134+
expect(actual.request.body.hparamsLimit).toEqual(100);
135+
expect(actual.request.body.includeMetrics).toBeFalse();
136+
});
125137
});
126138

127139
describe('fetchSessionGroups', () => {
@@ -176,6 +188,25 @@ describe('HparamsDataSource Test', () => {
176188
expect(sessionGroups[0].sessions[0].name).toEqual('eid1/run_name_1');
177189
expect(sessionGroups[1].sessions[0].name).toEqual('eid2/run_name_2');
178190
});
191+
192+
it('adds hparams as colParams', () => {
193+
dataSource
194+
.fetchSessionGroups(
195+
['eid'],
196+
[
197+
buildHparamSpec({name: 'hparam1'}),
198+
buildHparamSpec({name: 'hparam2'}),
199+
]
200+
)
201+
.subscribe();
202+
const actual = httpMock.expectOne(
203+
'/experiment/eid/data/plugin/hparams/session_groups'
204+
);
205+
expect(actual.request.body.colParams).toEqual([
206+
{hparam: 'hparam1', includeInResult: true},
207+
{hparam: 'hparam2', includeInResult: true},
208+
]);
209+
});
179210
});
180211
});
181212

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

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import * as hparamsActions from './hparams_actions';
3939
import {HparamsDataSource} from './hparams_data_source';
4040
import {HparamSpec, SessionGroup} from '../types';
4141
import {RouteKind} from '../../app_routing/types';
42+
import {getNumDashboardHparamsToLoad} from './hparams_selectors';
4243

4344
/**
4445
* Effects for fetching the hparams data from the backend.
@@ -70,40 +71,51 @@ export class HparamsEffects {
7071
/** @export */
7172
loadHparamsData$ = createEffect(() => {
7273
return merge(this.navigated$, this.loadHparamsOnReload$).pipe(
73-
withLatestFrom(this.store.select(getActiveRoute)),
74+
withLatestFrom(
75+
this.store.select(getActiveRoute),
76+
this.store.select(getNumDashboardHparamsToLoad)
77+
),
7478
filter(
7579
([, activeRoute]) =>
7680
activeRoute?.routeKind === RouteKind.EXPERIMENT ||
7781
activeRoute?.routeKind === RouteKind.COMPARE_EXPERIMENT
7882
),
7983
throttleTime(10),
80-
switchMap(([experimentIds]) =>
81-
this.loadHparamsForExperiments(experimentIds)
84+
switchMap(([experimentIds, , numHparamsToLoad]) =>
85+
this.loadHparamsForExperiments(experimentIds, numHparamsToLoad)
8286
),
8387
map((resp) => hparamsActions.hparamsFetchSessionGroupsSucceeded(resp))
8488
);
8589
});
8690

87-
private loadHparamsForExperiments(experimentIds: string[]): Observable<{
91+
private loadHparamsForExperiments(
92+
experimentIds: string[],
93+
hparamsLimit: number
94+
): Observable<{
8895
hparamSpecs: HparamSpec[];
8996
sessionGroups: SessionGroup[];
9097
}> {
91-
return this.dataSource.fetchExperimentInfo(experimentIds).pipe(
92-
switchMap((hparamSpecs) => {
93-
return this.dataSource
94-
.fetchSessionGroups(experimentIds, hparamSpecs)
95-
.pipe(
96-
catchError((error) => {
97-
// HParam plugin return 400 when there are no hparams
98-
// for an experiment.
99-
if (error instanceof HttpErrorResponse && error.status === 400) {
100-
return of([] as SessionGroup[]);
101-
}
102-
return throwError(() => error);
103-
}),
104-
map((sessionGroups) => ({hparamSpecs, sessionGroups}))
105-
);
106-
})
107-
);
98+
return this.dataSource
99+
.fetchExperimentInfo(experimentIds, hparamsLimit)
100+
.pipe(
101+
switchMap((hparamSpecs) => {
102+
return this.dataSource
103+
.fetchSessionGroups(experimentIds, hparamSpecs)
104+
.pipe(
105+
catchError((error) => {
106+
// HParam plugin return 400 when there are no hparams
107+
// for an experiment.
108+
if (
109+
error instanceof HttpErrorResponse &&
110+
error.status === 400
111+
) {
112+
return of([] as SessionGroup[]);
113+
}
114+
return throwError(() => error);
115+
}),
116+
map((sessionGroups) => ({hparamSpecs, sessionGroups}))
117+
);
118+
})
119+
);
108120
}
109121
}

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

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {buildHparamSpec} from './testing';
2727
import {HparamSpec, RunStatus, SessionGroup} from '../types';
2828
import * as selectors from '../../selectors';
2929
import * as hparamsActions from './hparams_actions';
30+
import * as hparamsSelectors from './hparams_selectors';
3031
import * as appRoutingActions from '../../app_routing/actions';
3132
import * as coreActions from '../../core/actions';
3233
import {RouteKind} from '../../app_routing/types';
@@ -112,6 +113,10 @@ describe('hparams effects', () => {
112113
store.overrideSelector(selectors.getExperimentIdsFromRoute, [
113114
'expFromRoute',
114115
]);
116+
store.overrideSelector(
117+
hparamsSelectors.getNumDashboardHparamsToLoad,
118+
1111
119+
);
115120
store.refreshState();
116121
});
117122

@@ -130,9 +135,10 @@ describe('hparams effects', () => {
130135

131136
it('fetches data after navigation', () => {
132137
action.next(appRoutingActions.navigated({} as any));
133-
expect(dataSource.fetchExperimentInfo).toHaveBeenCalledWith([
134-
'expFromRoute',
135-
]);
138+
expect(dataSource.fetchExperimentInfo).toHaveBeenCalledWith(
139+
['expFromRoute'],
140+
1111
141+
);
136142
expect(dataSource.fetchSessionGroups).toHaveBeenCalledWith(
137143
['expFromRoute'],
138144
[buildHparamSpec({name: 'h1'})]
@@ -158,9 +164,10 @@ describe('hparams effects', () => {
158164

159165
it('fetches data on reload', () => {
160166
action.next(coreActions.reload());
161-
expect(dataSource.fetchExperimentInfo).toHaveBeenCalledWith([
162-
'expFromRoute',
163-
]);
167+
expect(dataSource.fetchExperimentInfo).toHaveBeenCalledWith(
168+
['expFromRoute'],
169+
1111
170+
);
164171
expect(dataSource.fetchSessionGroups).toHaveBeenCalledWith(
165172
['expFromRoute'],
166173
[buildHparamSpec({name: 'h1'})]
@@ -175,9 +182,10 @@ describe('hparams effects', () => {
175182

176183
it('fetches data on manualReload', () => {
177184
action.next(coreActions.manualReload());
178-
expect(dataSource.fetchExperimentInfo).toHaveBeenCalledWith([
179-
'expFromRoute',
180-
]);
185+
expect(dataSource.fetchExperimentInfo).toHaveBeenCalledWith(
186+
['expFromRoute'],
187+
1111
188+
);
181189
expect(dataSource.fetchSessionGroups).toHaveBeenCalledWith(
182190
['expFromRoute'],
183191
[buildHparamSpec({name: 'h1'})]

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import {Side} from '../../widgets/data_table/types';
1919
import * as actions from './hparams_actions';
2020
import {HparamsState} from './types';
2121

22+
const INITIAL_NUM_HPARAMS_TO_LOAD = 1000;
23+
2224
const initialState: HparamsState = {
2325
dashboardHparamSpecs: [],
2426
dashboardSessionGroups: [],
@@ -27,6 +29,8 @@ const initialState: HparamsState = {
2729
metrics: new Map(),
2830
},
2931
dashboardDisplayedHparamColumns: [],
32+
numDashboardHparamsToLoad: INITIAL_NUM_HPARAMS_TO_LOAD,
33+
numDashboardHparamsLoaded: 0,
3034
};
3135

3236
const reducer: ActionReducer<HparamsState, Action> = createReducer(
@@ -49,6 +53,7 @@ const reducer: ActionReducer<HparamsState, Action> = createReducer(
4953
...state,
5054
dashboardHparamSpecs: nextDashboardHparamSpecs,
5155
dashboardSessionGroups: nextDashboardSessionGroups,
56+
numDashboardHparamsLoaded: nextDashboardHparamSpecs.length,
5257
};
5358
}),
5459
on(actions.dashboardHparamFilterAdded, (state, action) => {

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,25 @@ describe('hparams/_redux/hparams_reducers_test', () => {
144144

145145
expect(state2.dashboardSessionGroups).toEqual([mockSessionGroup]);
146146
});
147+
148+
it('calculates numDashboardHparamsLoaded from action.hparamSpecs', () => {
149+
const state = buildHparamsState({
150+
numDashboardHparamsLoaded: 0,
151+
});
152+
const state2 = reducers(
153+
state,
154+
actions.hparamsFetchSessionGroupsSucceeded({
155+
hparamSpecs: [
156+
buildHparamSpec(),
157+
buildHparamSpec(),
158+
buildHparamSpec(),
159+
],
160+
sessionGroups: [],
161+
})
162+
);
163+
164+
expect(state2.numDashboardHparamsLoaded).toEqual(3);
165+
});
147166
});
148167

149168
describe('dashboardHparamFilterAdded', () => {

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,17 @@ export const getDashboardMetricsFilterMap = createSelector(
7171
return state.dashboardFilters.metrics;
7272
}
7373
);
74+
75+
export const getNumDashboardHparamsToLoad = createSelector(
76+
getHparamsState,
77+
(state) => {
78+
return state.numDashboardHparamsToLoad;
79+
}
80+
);
81+
82+
export const getNumDashboardHparamsLoaded = createSelector(
83+
getHparamsState,
84+
(state) => {
85+
return state.numDashboardHparamsLoaded;
86+
}
87+
);

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,4 +163,28 @@ describe('hparams/_redux/hparams_selectors_test', () => {
163163
]);
164164
});
165165
});
166+
167+
describe('#getNumDashboardHparamsToLoad', () => {
168+
it('returns dashboard specs', () => {
169+
const state = buildStateFromHparamsState(
170+
buildHparamsState({
171+
numDashboardHparamsToLoad: 5,
172+
})
173+
);
174+
175+
expect(selectors.getNumDashboardHparamsToLoad(state)).toEqual(5);
176+
});
177+
});
178+
179+
describe('#getNumDashboardHparamsToLoad', () => {
180+
it('returns dashboard specs', () => {
181+
const state = buildStateFromHparamsState(
182+
buildHparamsState({
183+
numDashboardHparamsLoaded: 22,
184+
})
185+
);
186+
187+
expect(selectors.getNumDashboardHparamsLoaded(state)).toEqual(22);
188+
});
189+
});
166190
});

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ export function buildHparamsState(
3939
},
4040
dashboardDisplayedHparamColumns:
4141
overrides.dashboardDisplayedHparamColumns ?? [],
42+
numDashboardHparamsLoaded: overrides.numDashboardHparamsLoaded ?? 0,
43+
numDashboardHparamsToLoad: overrides.numDashboardHparamsToLoad ?? 0,
4244
} as HparamsState;
4345
}
4446

0 commit comments

Comments
 (0)