Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HParams: Add effect to remove hparam filters when the related column is removed #6572

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions tensorboard/webapp/hparams/_redux/hparams_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,13 @@ export const dashboardMetricFilterAdded = createAction(
'[Hparams] Dashboard Metric Filter Added',
props<{name: string; filter: MetricFilter}>()
);

export const dashboardHparamFilterRemoved = createAction(
'[Hparams] Dashboard Hparam Filter Removed',
props<{name: string}>()
);

export const dashboardMetricFilterRemoved = createAction(
'[Hparams] Dashboard Metric Filter Removed',
props<{name: string}>()
);
24 changes: 24 additions & 0 deletions tensorboard/webapp/hparams/_redux/hparams_reducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,30 @@ const reducer: ActionReducer<HparamsState, Action> = createReducer(
const nextMetricFilters = new Map(state.dashboardFilters.metrics);
nextMetricFilters.set(action.name, action.filter);

return {
...state,
dashboardFilters: {
...state.dashboardFilters,
metrics: nextMetricFilters,
},
};
}),
on(actions.dashboardHparamFilterRemoved, (state, action) => {
const nextHparamFilters = new Map(state.dashboardFilters.hparams);
nextHparamFilters.delete(action.name);

return {
...state,
dashboardFilters: {
...state.dashboardFilters,
hparams: nextHparamFilters,
},
};
}),
on(actions.dashboardMetricFilterRemoved, (state, action) => {
const nextMetricFilters = new Map(state.dashboardFilters.metrics);
nextMetricFilters.delete(action.name);

return {
...state,
dashboardFilters: {
Expand Down
110 changes: 110 additions & 0 deletions tensorboard/webapp/hparams/_redux/hparams_reducers_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -752,4 +752,114 @@ describe('hparams/_redux/hparams_reducers_test', () => {
});
});
});

describe('dashboardHparamFilterRemoved', () => {
it('removes entry from dashboardFilters', () => {
const state = buildHparamsState({
dashboardFilters: {
hparams: new Map([
[
'hparam1',
{
type: DomainType.INTERVAL,
includeUndefined: true,
minValue: 5,
maxValue: 10,
filterLowerBound: 5,
filterUpperBound: 10,
},
],
[
'hparam2',
{
type: DomainType.INTERVAL,
includeUndefined: true,
minValue: 2,
maxValue: 20,
filterLowerBound: 2,
filterUpperBound: 20,
},
],
]),
},
});
const state2 = reducers(
state,
actions.dashboardHparamFilterRemoved({
name: 'hparam1',
})
);
expect(state2.dashboardFilters).toEqual({
hparams: new Map([
[
'hparam2',
{
type: DomainType.INTERVAL,
includeUndefined: true,
minValue: 2,
maxValue: 20,
filterLowerBound: 2,
filterUpperBound: 20,
},
],
]),
metrics: new Map(),
});
});
});

describe('dashboardMetricFilterRemoved', () => {
it('removes entry from dashboardFilters', () => {
const state = buildHparamsState({
dashboardFilters: {
metrics: new Map([
[
'metric 1',
{
type: DomainType.INTERVAL,
includeUndefined: true,
minValue: 5,
maxValue: 10,
filterLowerBound: 5,
filterUpperBound: 10,
},
],
[
'metric 2',
{
type: DomainType.INTERVAL,
includeUndefined: true,
minValue: 2,
maxValue: 20,
filterLowerBound: 2,
filterUpperBound: 20,
},
],
]),
},
});
const state2 = reducers(
state,
actions.dashboardMetricFilterRemoved({
name: 'metric 1',
})
);
expect(state2.dashboardFilters).toEqual({
hparams: new Map(),
metrics: new Map([
[
'metric 2',
{
type: DomainType.INTERVAL,
includeUndefined: true,
minValue: 2,
maxValue: 20,
filterLowerBound: 2,
filterUpperBound: 20,
},
],
]),
});
});
});
});
4 changes: 4 additions & 0 deletions tensorboard/webapp/runs/effects/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ tf_ng_module(
"//tensorboard/webapp/app_routing:types",
"//tensorboard/webapp/app_routing/actions",
"//tensorboard/webapp/core/actions",
"//tensorboard/webapp/hparams/_redux:hparams_actions",
"//tensorboard/webapp/runs:types",
"//tensorboard/webapp/runs/actions",
"//tensorboard/webapp/runs/data_source",
"//tensorboard/webapp/types",
"//tensorboard/webapp/widgets/data_table:types",
"@npm//@angular/core",
"@npm//@ngrx/effects",
"@npm//@ngrx/store",
Expand All @@ -43,11 +45,13 @@ tf_ts_library(
"//tensorboard/webapp/app_routing:testing",
"//tensorboard/webapp/app_routing:types",
"//tensorboard/webapp/core/actions",
"//tensorboard/webapp/hparams/_redux:hparams_actions",
"//tensorboard/webapp/runs/actions",
"//tensorboard/webapp/runs/data_source",
"//tensorboard/webapp/runs/data_source:testing",
"//tensorboard/webapp/testing:utils",
"//tensorboard/webapp/types",
"//tensorboard/webapp/widgets/data_table:types",
"@npm//@ngrx/effects",
"@npm//@ngrx/store",
"@npm//@types/jasmine",
Expand Down
32 changes: 32 additions & 0 deletions tensorboard/webapp/runs/effects/runs_effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,14 @@ import {
} from '../../selectors';
import {DataLoadState, LoadState} from '../../types/data';
import * as actions from '../actions';
import * as hparamsActions from '../../hparams/_redux/hparams_actions';
import {
HparamsAndMetadata,
Run,
RunsDataSource,
} from '../data_source/runs_data_source_types';
import {ExperimentIdToRunsAndMetadata} from '../types';
import {ColumnHeaderType} from '../../widgets/data_table/types';

/**
* Runs effect for fetching data from the backend.
Expand Down Expand Up @@ -172,6 +174,36 @@ export class RunsEffects {
{dispatch: false}
);

/**
* Removes hparam filter when column is removed.
*
* @export
*/
removeHparamFilterWhenColumnIsRemoved$ = createEffect(
() =>
this.actions$.pipe(
ofType(actions.runsTableHeaderRemoved),
tap(({header}) => {
if (header.type === ColumnHeaderType.HPARAM) {
this.store.dispatch(
hparamsActions.dashboardHparamFilterRemoved({
name: header.name,
})
);
return;
}
if (header.type === ColumnHeaderType.METRIC) {
this.store.dispatch(
hparamsActions.dashboardMetricFilterRemoved({
name: header.name,
})
);
}
})
),
{dispatch: false}
);

/**
* IMPORTANT: actions are dispatched even when there are no experiments to
* fetch.
Expand Down
48 changes: 48 additions & 0 deletions tensorboard/webapp/runs/effects/runs_effects_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
import {RouteKind} from '../../app_routing/types';
import {State} from '../../app_state';
import * as coreActions from '../../core/actions';
import * as hparamsActions from '../../hparams/_redux/hparams_actions';
import {
getActiveRoute,
getExperimentIdsFromRoute,
Expand All @@ -42,6 +43,7 @@ import {
TestingRunsDataSource,
} from '../data_source/testing';
import {RunsEffects} from './index';
import {ColumnHeaderType} from '../../widgets/data_table/types';

function createRun(override: Partial<Run> = {}) {
return {
Expand Down Expand Up @@ -947,4 +949,50 @@ describe('runs_effects', () => {
});
});
});

describe('removeHparamFilterWhenColumnIsRemoved$', () => {
beforeEach(() => {
effects.removeHparamFilterWhenColumnIsRemoved$.subscribe(() => {});
});

it('dispatches dashboardHparamFilterRemoved when column type is hparam', () => {
action.next(
actions.runsTableHeaderRemoved({
header: {
type: ColumnHeaderType.HPARAM,
name: 'some_hparam',
enabled: true,
displayName: 'Some Hparam',
},
})
);
store.refreshState();

expect(actualActions).toEqual([
hparamsActions.dashboardHparamFilterRemoved({
name: 'some_hparam',
}),
]);
});

it('dispatches dashboardMetricFilterRemoved when column type is metric', () => {
action.next(
actions.runsTableHeaderRemoved({
header: {
type: ColumnHeaderType.METRIC,
name: 'some_metric',
enabled: true,
displayName: 'Some Metric',
},
})
);
store.refreshState();

expect(actualActions).toEqual([
hparamsActions.dashboardMetricFilterRemoved({
name: 'some_metric',
}),
]);
});
});
});
1 change: 1 addition & 0 deletions tensorboard/webapp/widgets/data_table/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export enum ColumnHeaderType {
MEAN = 'MEAN',
RAW_CHANGE = 'RAW_CHANGE',
HPARAM = 'HPARAM',
METRIC = 'METRIC',
CUSTOM = 'CUSTOM',
}

Expand Down