diff --git a/tensorboard/webapp/metrics/store/metrics_initial_state_provider.ts b/tensorboard/webapp/metrics/store/metrics_initial_state_provider.ts index a4e308a35e..02ae9522e3 100644 --- a/tensorboard/webapp/metrics/store/metrics_initial_state_provider.ts +++ b/tensorboard/webapp/metrics/store/metrics_initial_state_provider.ts @@ -17,7 +17,7 @@ import {InjectionToken} from '@angular/core'; import {StoreConfig} from '@ngrx/store'; import {INITIAL_STATE} from './metrics_reducers'; -import {MetricsState} from './metrics_types'; +import {MetricsSettings, MetricsState} from './metrics_types'; /** @typehack */ import * as _typeHackStore from '@ngrx/store'; @@ -26,11 +26,11 @@ export const METRICS_STORE_CONFIG_TOKEN = new InjectionToken< >('Metrics Store Config'); export const METRICS_INITIAL_SETTINGS = new InjectionToken< - StoreConfig + StoreConfig >('Metrics Initial Settings Config'); export function getConfig( - settings: MetricsState['settings'] + settings: MetricsSettings ): StoreConfig { if (!settings) { return { diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index c1f954a350..19b15350d1 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -245,6 +245,7 @@ const {initialState, reducers: routeContextReducer} = createRouteContextedState( images: {}, }, settings: METRICS_SETTINGS_DEFAULT, + settingOverrides: {}, visibleCards: new Set(), } as MetricsRoutelessState, @@ -316,7 +317,7 @@ const reducer = createReducer( ); const hydratedSmoothing = hydratedState.metrics.smoothing; - let newSettings = state.settings; + let newSettings = state.settingOverrides; if (Number.isFinite(hydratedSmoothing) && hydratedSmoothing !== null) { const newSmoothing = Math.max( @@ -327,7 +328,7 @@ const reducer = createReducer( ) ); newSettings = { - ...state.settings, + ...state.settingOverrides, scalarSmoothing: newSmoothing, }; } @@ -335,7 +336,7 @@ const reducer = createReducer( return { ...state, ...resolvedResult, - settings: newSettings, + settingOverrides: newSettings, }; }), on(actions.fetchPersistedSettingsSucceeded, (state, {partialSettings}) => { @@ -447,26 +448,29 @@ const reducer = createReducer( on(actions.metricsChangeTooltipSort, (state, {sort}) => { return { ...state, - settings: { - ...state.settings, + settingOverrides: { + ...state.settingOverrides, tooltipSort: sort, }, }; }), on(actions.metricsToggleIgnoreOutliers, (state) => { + const nextIgnoreOutliers = !( + state.settingOverrides.ignoreOutliers ?? state.settings.ignoreOutliers + ); return { ...state, - settings: { - ...state.settings, - ignoreOutliers: !state.settings.ignoreOutliers, + settingOverrides: { + ...state.settingOverrides, + ignoreOutliers: nextIgnoreOutliers, }, }; }), on(actions.metricsChangeXAxisType, (state, {xAxisType}) => { return { ...state, - settings: { - ...state.settings, + settingOverrides: { + ...state.settingOverrides, xAxisType, }, }; @@ -474,27 +478,30 @@ const reducer = createReducer( on(actions.metricsChangeScalarSmoothing, (state, {smoothing}) => { return { ...state, - settings: { - ...state.settings, + settingOverrides: { + ...state.settingOverrides, scalarSmoothing: smoothing, }, }; }), on(actions.metricsScalarPartitionNonMonotonicXToggled, (state) => { + const nextScalarPartitionNonMonotonicX = !( + state.settingOverrides.scalarPartitionNonMonotonicX ?? + state.settings.scalarPartitionNonMonotonicX + ); return { ...state, - settings: { - ...state.settings, - scalarPartitionNonMonotonicX: !state.settings - .scalarPartitionNonMonotonicX, + settingOverrides: { + ...state.settingOverrides, + scalarPartitionNonMonotonicX: nextScalarPartitionNonMonotonicX, }, }; }), on(actions.metricsChangeImageBrightness, (state, {brightnessInMilli}) => { return { ...state, - settings: { - ...state.settings, + settingOverrides: { + ...state.settingOverrides, imageBrightnessInMilli: brightnessInMilli, }, }; @@ -502,8 +509,8 @@ const reducer = createReducer( on(actions.metricsChangeImageContrast, (state, {contrastInMilli}) => { return { ...state, - settings: { - ...state.settings, + settingOverrides: { + ...state.settingOverrides, imageContrastInMilli: contrastInMilli, }, }; @@ -511,35 +518,39 @@ const reducer = createReducer( on(actions.metricsResetImageBrightness, (state) => { return { ...state, - settings: { - ...state.settings, - imageBrightnessInMilli: initialState.settings.imageBrightnessInMilli, + settingOverrides: { + ...state.settingOverrides, + imageBrightnessInMilli: undefined, }, }; }), on(actions.metricsResetImageContrast, (state) => { return { ...state, - settings: { - ...state.settings, - imageContrastInMilli: initialState.settings.imageContrastInMilli, + settingOverrides: { + ...state.settingOverrides, + imageContrastInMilli: undefined, }, }; }), on(actions.metricsToggleImageShowActualSize, (state) => { + const nextImageShowActualSize = !( + state.settingOverrides.imageShowActualSize ?? + state.settings.imageShowActualSize + ); return { ...state, - settings: { - ...state.settings, - imageShowActualSize: !state.settings.imageShowActualSize, + settingOverrides: { + ...state.settingOverrides, + imageShowActualSize: nextImageShowActualSize, }, }; }), on(actions.metricsChangeHistogramMode, (state, {histogramMode}) => { return { ...state, - settings: { - ...state.settings, + settingOverrides: { + ...state.settingOverrides, histogramMode, }, }; diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index aff393ff71..7ee326ea79 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -538,6 +538,9 @@ describe('metrics reducers', () => { it('changes tooltipSort on metricsChangeTooltipSort', () => { const prevState = buildMetricsState({ settings: buildMetricsSettingsState({ + tooltipSort: TooltipSort.DEFAULT, + }), + settingOverrides: buildMetricsSettingsState({ tooltipSort: TooltipSort.ASCENDING, }), }); @@ -545,7 +548,8 @@ describe('metrics reducers', () => { prevState, actions.metricsChangeTooltipSort({sort: TooltipSort.NEAREST}) ); - expect(nextState.settings.tooltipSort).toBe(TooltipSort.NEAREST); + expect(nextState.settings.tooltipSort).toBe(TooltipSort.DEFAULT); + expect(nextState.settingOverrides.tooltipSort).toBe(TooltipSort.NEAREST); }); it('changes ignoreOutliers on metricsToggleIgnoreOutliers', () => { @@ -553,12 +557,14 @@ describe('metrics reducers', () => { settings: buildMetricsSettingsState({ ignoreOutliers: true, }), + settingOverrides: {}, }); const nextState = reducers( prevState, actions.metricsToggleIgnoreOutliers() ); - expect(nextState.settings.ignoreOutliers).toBe(false); + expect(nextState.settings.ignoreOutliers).toBe(true); + expect(nextState.settingOverrides.ignoreOutliers).toBe(false); }); it('changes xAxisType on metricsChangeXAxisType', () => { @@ -566,23 +572,28 @@ describe('metrics reducers', () => { settings: buildMetricsSettingsState({ xAxisType: XAxisType.STEP, }), + settingOverrides: {}, }); const nextState = reducers( prevState, actions.metricsChangeXAxisType({xAxisType: XAxisType.WALL_TIME}) ); - expect(nextState.settings.xAxisType).toBe(XAxisType.WALL_TIME); + expect(nextState.settingOverrides.xAxisType).toBe(XAxisType.WALL_TIME); }); it('changes scalarSmoothing on metricsChangeScalarSmoothing', () => { const prevState = buildMetricsState({ settings: buildMetricsSettingsState({scalarSmoothing: 0.3}), + settingOverrides: { + scalarSmoothing: 0.5, + }, }); const nextState = reducers( prevState, actions.metricsChangeScalarSmoothing({smoothing: 0.1}) ); - expect(nextState.settings.scalarSmoothing).toBe(0.1); + expect(nextState.settings.scalarSmoothing).toBe(0.3); + expect(nextState.settingOverrides.scalarSmoothing).toBe(0.1); }); it('toggles Partition X on metricsScalarPartitionNonMonotonicXToggled', () => { @@ -590,18 +601,19 @@ describe('metrics reducers', () => { settings: buildMetricsSettingsState({ scalarPartitionNonMonotonicX: true, }), + settingOverrides: {}, }); const state2 = reducers( state1, actions.metricsScalarPartitionNonMonotonicXToggled() ); - expect(state2.settings.scalarPartitionNonMonotonicX).toBe(false); + expect(state2.settingOverrides.scalarPartitionNonMonotonicX).toBe(false); const state3 = reducers( state2, actions.metricsScalarPartitionNonMonotonicXToggled() ); - expect(state3.settings.scalarPartitionNonMonotonicX).toBe(true); + expect(state3.settingOverrides.scalarPartitionNonMonotonicX).toBe(true); }); it('changes imageBrightnessInMilli on metricsChangeImageBrightness', () => { @@ -609,12 +621,13 @@ describe('metrics reducers', () => { settings: buildMetricsSettingsState({ imageBrightnessInMilli: 300, }), + settingOverrides: {}, }); const nextState = reducers( prevState, actions.metricsChangeImageBrightness({brightnessInMilli: 1000}) ); - expect(nextState.settings.imageBrightnessInMilli).toBe(1000); + expect(nextState.settingOverrides.imageBrightnessInMilli).toBe(1000); }); it('changes imageContrastInMilli on metricsChangeImageContrast', () => { @@ -622,12 +635,13 @@ describe('metrics reducers', () => { settings: buildMetricsSettingsState({ imageContrastInMilli: 200, }), + settingOverrides: {}, }); const nextState = reducers( prevState, actions.metricsChangeImageContrast({contrastInMilli: 500}) ); - expect(nextState.settings.imageContrastInMilli).toBe(500); + expect(nextState.settingOverrides.imageContrastInMilli).toBe(500); }); it('resets imageBrightnessInMilli', () => { @@ -635,12 +649,16 @@ describe('metrics reducers', () => { settings: buildMetricsSettingsState({ imageBrightnessInMilli: 300, }), + settingOverrides: { + imageBrightnessInMilli: 0, + }, }); const nextState = reducers( prevState, actions.metricsResetImageBrightness() ); - expect(nextState.settings.imageBrightnessInMilli).toBe(1000); + expect(nextState.settings.imageBrightnessInMilli).toBe(300); + expect(nextState.settingOverrides.imageBrightnessInMilli).toBe(undefined); }); it('resets imageContrastInMilli', () => { @@ -648,12 +666,16 @@ describe('metrics reducers', () => { settings: buildMetricsSettingsState({ imageContrastInMilli: 300, }), + settingOverrides: { + imageContrastInMilli: 5000, + }, }); const nextState = reducers( prevState, actions.metricsResetImageContrast() ); - expect(nextState.settings.imageContrastInMilli).toBe(1000); + expect(nextState.settings.imageContrastInMilli).toBe(300); + expect(nextState.settingOverrides.imageContrastInMilli).toBe(undefined); }); it('changes imageShowActualSize on metricsToggleImageShowActualSize', () => { @@ -661,12 +683,13 @@ describe('metrics reducers', () => { settings: buildMetricsSettingsState({ imageShowActualSize: true, }), + settingOverrides: {}, }); const nextState = reducers( prevState, actions.metricsToggleImageShowActualSize() ); - expect(nextState.settings.imageShowActualSize).toBe(false); + expect(nextState.settingOverrides.imageShowActualSize).toBe(false); }); it('changes histogramMode on metricsChangeHistogramMode', () => { @@ -674,6 +697,7 @@ describe('metrics reducers', () => { settings: buildMetricsSettingsState({ histogramMode: HistogramMode.OFFSET, }), + settingOverrides: {}, }); const nextState = reducers( prevState, @@ -681,7 +705,9 @@ describe('metrics reducers', () => { histogramMode: HistogramMode.OVERLAY, }) ); - expect(nextState.settings.histogramMode).toBe(HistogramMode.OVERLAY); + expect(nextState.settingOverrides.histogramMode).toBe( + HistogramMode.OVERLAY + ); }); }); @@ -1614,7 +1640,8 @@ describe('metrics reducers', () => { describe('smoothing hydration', () => { it('rehydrates the smoothing state', () => { const beforeState = buildMetricsState({ - settings: buildMetricsSettingsState({scalarSmoothing: 0.3}), + settings: buildMetricsSettingsState({scalarSmoothing: 1}), + settingOverrides: {scalarSmoothing: 0.5}, }); const action = routingActions.stateRehydratedFromUrl({ routeKind: RouteKind.EXPERIMENT, @@ -1622,12 +1649,13 @@ describe('metrics reducers', () => { }); const nextState = reducers(beforeState, action); - expect(nextState.settings.scalarSmoothing).toBe(0.1); + expect(nextState.settings.scalarSmoothing).toBe(1); + expect(nextState.settingOverrides.scalarSmoothing).toBe(0.1); }); it('keeps old state when the rehydrated state is null', () => { const beforeState = buildMetricsState({ - settings: buildMetricsSettingsState({scalarSmoothing: 0.3}), + settingOverrides: {scalarSmoothing: 0.5}, }); const action = routingActions.stateRehydratedFromUrl({ routeKind: RouteKind.EXPERIMENT, @@ -1635,12 +1663,25 @@ describe('metrics reducers', () => { }); const nextState = reducers(beforeState, action); - expect(nextState.settings.scalarSmoothing).toBe(0.3); + expect(nextState.settingOverrides.scalarSmoothing).toBe(0.5); + }); + + it('keeps old state when the rehydrated state is null (empty override)', () => { + const beforeState = buildMetricsState({ + settingOverrides: {}, + }); + const action = routingActions.stateRehydratedFromUrl({ + routeKind: RouteKind.EXPERIMENT, + partialState: {metrics: {pinnedCards: [], smoothing: null}}, + }); + const nextState = reducers(beforeState, action); + + expect(nextState.settingOverrides.scalarSmoothing).toBe(undefined); }); it('keeps old state when the rehydrated state is NaN', () => { const beforeState = buildMetricsState({ - settings: buildMetricsSettingsState({scalarSmoothing: 0.3}), + settingOverrides: buildMetricsSettingsState({scalarSmoothing: 0.3}), }); const action = routingActions.stateRehydratedFromUrl({ routeKind: RouteKind.EXPERIMENT, @@ -1648,12 +1689,12 @@ describe('metrics reducers', () => { }); const nextState = reducers(beforeState, action); - expect(nextState.settings.scalarSmoothing).toBe(0.3); + expect(nextState.settingOverrides.scalarSmoothing).toBe(0.3); }); it('clips value to 0', () => { const beforeState = buildMetricsState({ - settings: buildMetricsSettingsState({scalarSmoothing: 0.3}), + settingOverrides: buildMetricsSettingsState({scalarSmoothing: 0.3}), }); const action = routingActions.stateRehydratedFromUrl({ routeKind: RouteKind.EXPERIMENT, @@ -1661,12 +1702,12 @@ describe('metrics reducers', () => { }); const nextState = reducers(beforeState, action); - expect(nextState.settings.scalarSmoothing).toBe(0); + expect(nextState.settingOverrides.scalarSmoothing).toBe(0); }); it('clips value to 0.999', () => { const beforeState = buildMetricsState({ - settings: buildMetricsSettingsState({scalarSmoothing: 0.3}), + settingOverrides: buildMetricsSettingsState({scalarSmoothing: 0.3}), }); const action = routingActions.stateRehydratedFromUrl({ routeKind: RouteKind.EXPERIMENT, @@ -1674,12 +1715,12 @@ describe('metrics reducers', () => { }); const nextState = reducers(beforeState, action); - expect(nextState.settings.scalarSmoothing).toBe(0.999); + expect(nextState.settingOverrides.scalarSmoothing).toBe(0.999); }); it('rounds to the 3 significant digits to prevent weird numbers', () => { const beforeState = buildMetricsState({ - settings: buildMetricsSettingsState({scalarSmoothing: 0.3}), + settingOverrides: buildMetricsSettingsState({scalarSmoothing: 0.3}), }); const action = routingActions.stateRehydratedFromUrl({ routeKind: RouteKind.EXPERIMENT, @@ -1687,18 +1728,21 @@ describe('metrics reducers', () => { }); const nextState = reducers(beforeState, action); - expect(nextState.settings.scalarSmoothing).toBe(0.232); + expect(nextState.settingOverrides.scalarSmoothing).toBe(0.232); }); }); describe('#fetchPersistedSettingsSucceeded', () => { - it('adds partial state from the action to the settings', () => { + it('adds partial state from the action to the (default) settings', () => { const beforeState = buildMetricsState({ settings: buildMetricsSettingsState({ scalarSmoothing: 0.3, ignoreOutliers: false, tooltipSort: TooltipSort.ASCENDING, }), + settingOverrides: { + scalarSmoothing: 0.5, + }, }); const nextState = reducers( @@ -1714,6 +1758,7 @@ describe('metrics reducers', () => { expect(nextState.settings.scalarSmoothing).toBe(0); expect(nextState.settings.ignoreOutliers).toBe(true); expect(nextState.settings.tooltipSort).toBe(TooltipSort.ASCENDING); + expect(nextState.settingOverrides.scalarSmoothing).toBe(0.5); }); }); }); diff --git a/tensorboard/webapp/metrics/store/metrics_selectors.ts b/tensorboard/webapp/metrics/store/metrics_selectors.ts index e5796148bc..04614761c9 100644 --- a/tensorboard/webapp/metrics/store/metrics_selectors.ts +++ b/tensorboard/webapp/metrics/store/metrics_selectors.ts @@ -13,27 +13,27 @@ See the License for the specific language governing permissions and limitations under the License. ==============================================================================*/ import {createFeatureSelector, createSelector} from '@ngrx/store'; -import {DataLoadState} from '../../types/data'; import {State} from '../../app_state'; +import {DataLoadState} from '../../types/data'; import {DeepReadonly} from '../../util/types'; import { CardId, CardIdWithMetadata, - CardUniqueInfo, CardMetadata, + CardUniqueInfo, HistogramMode, NonPinnedCardId, PinnedCardId, TooltipSort, XAxisType, } from '../internal_types'; - import * as storeUtils from './metrics_store_internal_utils'; import { CardMetadataMap, - METRICS_FEATURE_KEY, + MetricsSettings, MetricsState, + METRICS_FEATURE_KEY, RunToSeries, TagMetadata, } from './metrics_types'; @@ -259,12 +259,24 @@ export const getCanCreateNewPins = createSelector( const selectSettings = createSelector( selectMetricsState, - (state): MetricsState['settings'] => state.settings + (state): MetricsSettings => { + return { + ...state.settings, + ...state.settingOverrides, + }; + } ); /** * Settings. */ +export const getMetricsSettingOverrides = createSelector( + selectMetricsState, + (state): Partial => { + return state.settingOverrides; + } +); + export const getMetricsTooltipSort = createSelector( selectSettings, (settings): TooltipSort => settings.tooltipSort diff --git a/tensorboard/webapp/metrics/store/metrics_types.ts b/tensorboard/webapp/metrics/store/metrics_types.ts index 69f1971cb2..8948133d8d 100644 --- a/tensorboard/webapp/metrics/store/metrics_types.ts +++ b/tensorboard/webapp/metrics/store/metrics_types.ts @@ -153,36 +153,41 @@ export interface MetricsRoutefulState { tagGroupExpanded: Map; } +export interface MetricsSettings { + tooltipSort: TooltipSort; + ignoreOutliers: boolean; + xAxisType: XAxisType; + scalarSmoothing: number; + /** + * https://github.com/tensorflow/tensorboard/issues/3732 + * + * When a ML job restarts from a checkpoint or if a user writes to the same logdir + * with overlapping steps, TensorBoard shows a zig-zag lines which tend to confuse + * users. This setting guarantees that each line forms a monotonic increases in x-axis + * by creating a pseudo-runs by partitioning the runs on the client side. In the + * future, we may fix this at the log writing, reading, or backend response time. + */ + scalarPartitionNonMonotonicX: boolean; + /** + * A non-negative, unitless number. A value of 5000 corresponds to 500% + * increased brightness from normal. + */ + imageBrightnessInMilli: number; + /** + * A non-negative, unitless number. A value of 5000 corresponds to 500% + * increased contrast from normal. + */ + imageContrastInMilli: number; + imageShowActualSize: boolean; + histogramMode: HistogramMode; +} + export interface MetricsRoutelessState { timeSeriesData: TimeSeriesData; - settings: { - tooltipSort: TooltipSort; - ignoreOutliers: boolean; - xAxisType: XAxisType; - scalarSmoothing: number; - /** - * https://github.com/tensorflow/tensorboard/issues/3732 - * - * When a ML job restarts from a checkpoint or if a user writes to the same logdir - * with overlapping steps, TensorBoard shows a zig-zag lines which tend to confuse - * users. This setting guarantees that each line forms a monotonic increases in x-axis - * by creating a pseudo-runs by partitioning the runs on the client side. In the - * future, we may fix this at the log writing, reading, or backend response time. - */ - scalarPartitionNonMonotonicX: boolean; - /** - * A non-negative, unitless number. A value of 5000 corresponds to 500% - * increased brightness from normal. - */ - imageBrightnessInMilli: number; - /** - * A non-negative, unitless number. A value of 5000 corresponds to 500% - * increased contrast from normal. - */ - imageContrastInMilli: number; - imageShowActualSize: boolean; - histogramMode: HistogramMode; - }; + // Default settings. For the legacy reasons, we cannot change the name of the + // prop. It either is set by application or a user via settings storage. + settings: MetricsSettings; + settingOverrides: Partial; visibleCards: Set; } @@ -195,7 +200,7 @@ export interface State { [METRICS_FEATURE_KEY]?: MetricsState; } -export const METRICS_SETTINGS_DEFAULT: MetricsState['settings'] = { +export const METRICS_SETTINGS_DEFAULT: MetricsSettings = { tooltipSort: TooltipSort.DEFAULT, ignoreOutliers: true, xAxisType: XAxisType.STEP, diff --git a/tensorboard/webapp/metrics/testing.ts b/tensorboard/webapp/metrics/testing.ts index 9b46bc901a..b02251a1f3 100644 --- a/tensorboard/webapp/metrics/testing.ts +++ b/tensorboard/webapp/metrics/testing.ts @@ -37,14 +37,14 @@ import { TimeSeriesData, } from './store'; import * as selectors from './store/metrics_selectors'; -import {RunToSeries, StepDatum} from './store/metrics_types'; +import {MetricsSettings, RunToSeries, StepDatum} from './store/metrics_types'; import {CardId, CardMetadata, TooltipSort, XAxisType} from './types'; /** @typehack */ import * as _typeHackRxjs from 'rxjs'; export function buildMetricsSettingsState( - overrides?: Partial -): MetricsState['settings'] { + overrides?: Partial +): MetricsSettings { return { tooltipSort: TooltipSort.NEAREST, ignoreOutliers: false, @@ -82,6 +82,7 @@ function buildBlankState(): MetricsState { images: {}, }, settings: buildMetricsSettingsState(), + settingOverrides: {}, cardList: [], cardToPinnedCopy: new Map(), pinnedCardToOriginal: new Map(), diff --git a/tensorboard/webapp/metrics/types.ts b/tensorboard/webapp/metrics/types.ts index 139b878e66..bc2af3751a 100644 --- a/tensorboard/webapp/metrics/types.ts +++ b/tensorboard/webapp/metrics/types.ts @@ -13,4 +13,3 @@ See the License for the specific language governing permissions and limitations under the License. ==============================================================================*/ export * from './internal_types'; -export {METRICS_SETTINGS_DEFAULT} from './store/metrics_types'; diff --git a/tensorboard/webapp/routes/dashboard_deeplink_provider.ts b/tensorboard/webapp/routes/dashboard_deeplink_provider.ts index 3a84ceb83d..6782751ced 100644 --- a/tensorboard/webapp/routes/dashboard_deeplink_provider.ts +++ b/tensorboard/webapp/routes/dashboard_deeplink_provider.ts @@ -15,7 +15,7 @@ limitations under the License. import {Injectable} from '@angular/core'; import {Store} from '@ngrx/store'; import {combineLatest, Observable} from 'rxjs'; -import {map} from 'rxjs/operators'; +import {filter, map} from 'rxjs/operators'; import {DeepLinkProvider} from '../app_routing/deep_link_provider'; import {SerializableQueryParams} from '../app_routing/types'; @@ -25,9 +25,8 @@ import { isSampledPlugin, isSingleRunPlugin, } from '../metrics/data_source/types'; -import {CardUniqueInfo, METRICS_SETTINGS_DEFAULT} from '../metrics/types'; +import {CardUniqueInfo} from '../metrics/types'; import * as selectors from '../selectors'; -import {getMetricsScalarSmoothing} from '../selectors'; import { ENABLE_COLOR_GROUP_QUERY_PARAM_KEY, EXPERIMENTAL_PLUGIN_QUERY_PARAM_KEY, @@ -106,10 +105,17 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider { return combineLatest([ this.getMetricsPinnedCards(store), this.getFeatureFlagStates(store), - store.select(getMetricsScalarSmoothing).pipe( - map((smoothing) => { - if (smoothing === METRICS_SETTINGS_DEFAULT.scalarSmoothing) return []; - return [{key: SMOOTHING_KEY, value: String(smoothing)}]; + store.select(selectors.getMetricsSettingOverrides).pipe( + map((settingOverrides) => { + if (Number.isFinite(settingOverrides.scalarSmoothing)) { + return [ + { + key: SMOOTHING_KEY, + value: String(settingOverrides.scalarSmoothing), + }, + ]; + } + return []; }) ), ]).pipe( diff --git a/tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts b/tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts index f6758e90d8..cddc6804d1 100644 --- a/tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts +++ b/tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts @@ -20,7 +20,6 @@ import {skip} from 'rxjs/operators'; import {SerializableQueryParams} from '../app_routing/types'; import {State} from '../app_state'; import {PluginType} from '../metrics/data_source/types'; -import {METRICS_SETTINGS_DEFAULT} from '../metrics/types'; import {appStateFromMetricsState, buildMetricsState} from '../metrics/testing'; import * as selectors from '../selectors'; import {DashboardDeepLinkProvider} from './dashboard_deeplink_provider'; @@ -46,10 +45,7 @@ describe('core deeplink provider', () => { store.overrideSelector(selectors.getUnresolvedImportedPinnedCards, []); store.overrideSelector(selectors.getEnabledExperimentalPlugins, []); store.overrideSelector(selectors.getOverriddenFeatureFlags, {}); - store.overrideSelector( - selectors.getMetricsScalarSmoothing, - METRICS_SETTINGS_DEFAULT.scalarSmoothing - ); + store.overrideSelector(selectors.getMetricsSettingOverrides, {}); queryParamsSerialized = []; @@ -68,7 +64,9 @@ describe('core deeplink provider', () => { describe('time series', () => { describe('smoothing state', () => { it('serializes the smoothing state to the URL', () => { - store.overrideSelector(selectors.getMetricsScalarSmoothing, 0); + store.overrideSelector(selectors.getMetricsSettingOverrides, { + scalarSmoothing: 0, + }); store.refreshState(); expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual( @@ -81,11 +79,14 @@ describe('core deeplink provider', () => { ); }); - it('does not reflect default value to the URL', () => { - store.overrideSelector(selectors.getMetricsScalarSmoothing, 0.6); + it('does not reflect state when there is no override', () => { + store.overrideSelector(selectors.getMetricsSettingOverrides, {}); + store.refreshState(); - expect(queryParamsSerialized.length).toBe(0); + expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual( + [] + ); }); it('deserializes the state in the URL without much sanitization', () => {