Skip to content

Commit

Permalink
HParams: Stop fetching hparams data using run effects (#6561)
Browse files Browse the repository at this point in the history
## Motivation for features / changes
Now that we are fetching the hparams data using the newly created
hparams_effects (#6540) we no longer need to fetch them using the old
method.

Note that the experiment list SHOULD still use the old method.
  • Loading branch information
rileyajones authored Aug 30, 2023
1 parent 3a70adc commit f7680b4
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 5 deletions.
15 changes: 15 additions & 0 deletions tensorboard/webapp/app_routing/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@ export enum RouteKind {
NOT_SET,
}

export type DashboardRoute =
| RouteKind.EXPERIMENT
| RouteKind.COMPARE_EXPERIMENT
| RouteKind.CARD;

export function isDashboardRoute(
routeKind: RouteKind
): routeKind is DashboardRoute {
return (
routeKind === RouteKind.EXPERIMENT ||
routeKind === RouteKind.COMPARE_EXPERIMENT ||
routeKind === RouteKind.CARD
);
}

export const DEFAULT_EXPERIMENT_ID = 'defaultExperimentId';

/**
Expand Down
29 changes: 25 additions & 4 deletions tensorboard/webapp/runs/effects/runs_effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,27 @@ limitations under the License.
import {Injectable} from '@angular/core';
import {Actions, createEffect, ofType} from '@ngrx/effects';
import {Store} from '@ngrx/store';
import {forkJoin, merge, Observable, of, throwError} from 'rxjs';
import {forkJoin, merge, Observable, of, throwError, zip} from 'rxjs';
import {
catchError,
distinctUntilChanged,
filter,
map,
mergeMap,
switchMap,
take,
tap,
withLatestFrom,
} from 'rxjs/operators';
import {areSameRouteKindAndExperiments} from '../../app_routing';
import {navigated} from '../../app_routing/actions';
import {RouteKind} from '../../app_routing/types';
import {RouteKind, isDashboardRoute} from '../../app_routing/types';
import {State} from '../../app_state';
import * as coreActions from '../../core/actions';
import {
getActiveRoute,
getRouteKind,
getEnableHparamsInTimeSeries,
getExperimentIdsFromRoute,
getRuns,
getRunsLoadState,
Expand Down Expand Up @@ -264,15 +267,33 @@ export class RunsEffects {
);
}

private maybeFetchHparamsMetadata(
experimentId: string
): Observable<HparamsAndMetadata> {
return this.store.select(getEnableHparamsInTimeSeries).pipe(
withLatestFrom(this.store.select(getRouteKind)),
switchMap(([hparamsInTimeSeries, routeKind]) => {
if (hparamsInTimeSeries && isDashboardRoute(routeKind)) {
return of({
hparamSpecs: [],
metricSpecs: [],
runToHparamsAndMetrics: {},
});
}
return this.runsDataSource.fetchHparamsMetadata(experimentId);
})
);
}

private fetchRunsForExperiment(experimentId: string): Observable<{
fromRemote: true;
experimentId: string;
runs: Run[];
metadata: HparamsAndMetadata;
}> {
return forkJoin([
return zip([
this.runsDataSource.fetchRuns(experimentId),
this.runsDataSource.fetchHparamsMetadata(experimentId),
this.maybeFetchHparamsMetadata(experimentId),
]).pipe(
map(([runs, metadata]) => {
return {fromRemote: true, experimentId, runs, metadata};
Expand Down
15 changes: 14 additions & 1 deletion tensorboard/webapp/runs/effects/runs_effects_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ import {State} from '../../app_state';
import * as coreActions from '../../core/actions';
import {
getActiveRoute,
getEnableHparamsInTimeSeries,
getExperimentIdsFromRoute,
getRouteKind,
getRuns,
getRunsLoadState,
} from '../../selectors';
Expand Down Expand Up @@ -62,6 +64,7 @@ describe('runs_effects', () => {
let dispatchSpy: jasmine.Spy;
let actualActions: Action[];
let selectSpy: jasmine.Spy;
let fetchHparamsSpy: jasmine.Spy;

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

fetchHparamsMetadataSubjects = [];
spyOn(runsDataSource, 'fetchHparamsMetadata').and.callFake(() => {
fetchHparamsSpy = spyOn(runsDataSource, 'fetchHparamsMetadata');
fetchHparamsSpy.and.callFake(() => {
const subject = new ReplaySubject<HparamsAndMetadata>(1);
fetchHparamsMetadataSubjects.push(subject);
return subject;
Expand Down Expand Up @@ -231,6 +235,15 @@ describe('runs_effects', () => {
});
});

it('does not fetch hparam data when enableHparamsInTimeSeries is true when on a dashboard route', () => {
store.overrideSelector(getEnableHparamsInTimeSeries, true);
store.overrideSelector(getRouteKind, RouteKind.EXPERIMENT);
store.refreshState();

action.next(actions.runTableShown({experimentIds: ['a']}));
expect(fetchHparamsSpy).not.toHaveBeenCalled();
});

it('fires FAILED action when failed to fetch runs', () => {
action.next(actions.runTableShown({experimentIds: ['a']}));
const expectedExperimentId = 'a';
Expand Down

0 comments on commit f7680b4

Please sign in to comment.