Skip to content

Commit a0c241d

Browse files
authored
Revert HParams: Stop fetching hparams data using run effects (#6561) (#6571)
## Motivation for features / changes #6561 created issues when syncing internally. When we discovered the problem we tried to patch forward twice * #6566 * #6568 There was a couple of issues. * Improper support for tuples internally. * Internally `forkJoin` and `zip` work differently, it's not document how though. * The unapproved screenshot changes in [cl/563231334](http://cl/563231334) Googlers see [cl/563516101](http://cl/563516101) for confidence this ACTUALLY fixes the sync.
1 parent 48c4d24 commit a0c241d

File tree

3 files changed

+6
-57
lines changed

3 files changed

+6
-57
lines changed

tensorboard/webapp/app_routing/types.ts

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,21 +38,6 @@ export enum RouteKind {
3838
NOT_SET,
3939
}
4040

41-
export type DashboardRoute =
42-
| RouteKind.EXPERIMENT
43-
| RouteKind.COMPARE_EXPERIMENT
44-
| RouteKind.CARD;
45-
46-
export function isDashboardRoute(
47-
routeKind: RouteKind
48-
): routeKind is DashboardRoute {
49-
return (
50-
routeKind === RouteKind.EXPERIMENT ||
51-
routeKind === RouteKind.COMPARE_EXPERIMENT ||
52-
routeKind === RouteKind.CARD
53-
);
54-
}
55-
5641
export const DEFAULT_EXPERIMENT_ID = 'defaultExperimentId';
5742

5843
/**

tensorboard/webapp/runs/effects/runs_effects.ts

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,24 @@ limitations under the License.
1515
import {Injectable} from '@angular/core';
1616
import {Actions, createEffect, ofType} from '@ngrx/effects';
1717
import {Store} from '@ngrx/store';
18-
import {combineLatest, forkJoin, merge, Observable, of, throwError} from 'rxjs';
18+
import {forkJoin, merge, Observable, of, throwError} from 'rxjs';
1919
import {
2020
catchError,
2121
distinctUntilChanged,
2222
filter,
2323
map,
2424
mergeMap,
25-
switchMap,
2625
take,
2726
tap,
2827
withLatestFrom,
2928
} from 'rxjs/operators';
3029
import {areSameRouteKindAndExperiments} from '../../app_routing';
3130
import {navigated} from '../../app_routing/actions';
32-
import {RouteKind, isDashboardRoute} from '../../app_routing/types';
31+
import {RouteKind} from '../../app_routing/types';
3332
import {State} from '../../app_state';
3433
import * as coreActions from '../../core/actions';
3534
import {
3635
getActiveRoute,
37-
getRouteKind,
38-
getEnableHparamsInTimeSeries,
3936
getExperimentIdsFromRoute,
4037
getRuns,
4138
getRunsLoadState,
@@ -209,7 +206,7 @@ export class RunsEffects {
209206
}
210207
return this.maybeWaitForRunsAndGetRuns(experimentId);
211208
});
212-
return combineLatest(fetchOrGetRuns);
209+
return forkJoin(fetchOrGetRuns);
213210
}),
214211
map((runsAndMedataList) => {
215212
const newRunsAndMetadata = {} as ExperimentIdToRunsAndMetadata;
@@ -267,35 +264,15 @@ export class RunsEffects {
267264
);
268265
}
269266

270-
private maybeFetchHparamsMetadata(
271-
experimentId: string
272-
): Observable<HparamsAndMetadata> {
273-
return combineLatest([
274-
this.store.select(getEnableHparamsInTimeSeries),
275-
this.store.select(getRouteKind),
276-
]).pipe(
277-
switchMap(([hparamsInTimeSeries, routeKind]) => {
278-
if (hparamsInTimeSeries && isDashboardRoute(routeKind)) {
279-
return of({
280-
hparamSpecs: [],
281-
metricSpecs: [],
282-
runToHparamsAndMetrics: {},
283-
});
284-
}
285-
return this.runsDataSource.fetchHparamsMetadata(experimentId);
286-
})
287-
);
288-
}
289-
290267
private fetchRunsForExperiment(experimentId: string): Observable<{
291268
fromRemote: true;
292269
experimentId: string;
293270
runs: Run[];
294271
metadata: HparamsAndMetadata;
295272
}> {
296-
return combineLatest([
273+
return forkJoin([
297274
this.runsDataSource.fetchRuns(experimentId),
298-
this.maybeFetchHparamsMetadata(experimentId),
275+
this.runsDataSource.fetchHparamsMetadata(experimentId),
299276
]).pipe(
300277
map(([runs, metadata]) => {
301278
return {

tensorboard/webapp/runs/effects/runs_effects_test.ts

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@ import {State} from '../../app_state';
2828
import * as coreActions from '../../core/actions';
2929
import {
3030
getActiveRoute,
31-
getEnableHparamsInTimeSeries,
3231
getExperimentIdsFromRoute,
33-
getRouteKind,
3432
getRuns,
3533
getRunsLoadState,
3634
} from '../../selectors';
@@ -64,7 +62,6 @@ describe('runs_effects', () => {
6462
let dispatchSpy: jasmine.Spy;
6563
let actualActions: Action[];
6664
let selectSpy: jasmine.Spy;
67-
let fetchHparamsSpy: jasmine.Spy;
6865

6966
function flushFetchRuns(requestIndex: number, runs: Run[]) {
7067
expect(fetchRunsSubjects.length).toBeGreaterThan(requestIndex);
@@ -116,8 +113,7 @@ describe('runs_effects', () => {
116113
});
117114

118115
fetchHparamsMetadataSubjects = [];
119-
fetchHparamsSpy = spyOn(runsDataSource, 'fetchHparamsMetadata');
120-
fetchHparamsSpy.and.callFake(() => {
116+
spyOn(runsDataSource, 'fetchHparamsMetadata').and.callFake(() => {
121117
const subject = new ReplaySubject<HparamsAndMetadata>(1);
122118
fetchHparamsMetadataSubjects.push(subject);
123119
return subject;
@@ -235,15 +231,6 @@ describe('runs_effects', () => {
235231
});
236232
});
237233

238-
it('does not fetch hparam data when enableHparamsInTimeSeries is true when on a dashboard route', () => {
239-
store.overrideSelector(getEnableHparamsInTimeSeries, true);
240-
store.overrideSelector(getRouteKind, RouteKind.EXPERIMENT);
241-
store.refreshState();
242-
243-
action.next(actions.runTableShown({experimentIds: ['a']}));
244-
expect(fetchHparamsSpy).not.toHaveBeenCalled();
245-
});
246-
247234
it('fires FAILED action when failed to fetch runs', () => {
248235
action.next(actions.runTableShown({experimentIds: ['a']}));
249236
const expectedExperimentId = 'a';

0 commit comments

Comments
 (0)