From 17a28b7e456ead617a26049c307dc0c379748ccf Mon Sep 17 00:00:00 2001 From: Riley Jones Date: Thu, 15 Jun 2023 22:28:32 +0000 Subject: [PATCH] add redux logic --- tensorboard/webapp/BUILD | 2 + tensorboard/webapp/metrics/BUILD | 1 + tensorboard/webapp/metrics/actions/index.ts | 13 +- tensorboard/webapp/metrics/data_source/BUILD | 26 ++ .../card_interactions_data_source.ts | 43 ++++ .../card_interactions_data_source_module.ts | 22 ++ .../card_interactions_data_source_test.ts | 125 ++++++++++ tensorboard/webapp/metrics/effects/BUILD | 16 +- .../effects/card_interaction_effects.ts | 118 +++++++++ .../effects/card_interactions_effects_test.ts | 223 ++++++++++++++++++ .../webapp/metrics/store/metrics_reducers.ts | 86 +++++++ .../metrics/store/metrics_reducers_test.ts | 17 ++ .../webapp/metrics/store/metrics_selectors.ts | 73 +++++- .../metrics/store/metrics_selectors_test.ts | 88 ++++++- .../webapp/metrics/store/metrics_types.ts | 9 + tensorboard/webapp/metrics/testing.ts | 10 + 16 files changed, 866 insertions(+), 6 deletions(-) create mode 100644 tensorboard/webapp/metrics/data_source/card_interactions_data_source.ts create mode 100644 tensorboard/webapp/metrics/data_source/card_interactions_data_source_module.ts create mode 100644 tensorboard/webapp/metrics/data_source/card_interactions_data_source_test.ts create mode 100644 tensorboard/webapp/metrics/effects/card_interaction_effects.ts create mode 100644 tensorboard/webapp/metrics/effects/card_interactions_effects_test.ts diff --git a/tensorboard/webapp/BUILD b/tensorboard/webapp/BUILD index d167c713f4..ad28bea976 100644 --- a/tensorboard/webapp/BUILD +++ b/tensorboard/webapp/BUILD @@ -269,6 +269,7 @@ tf_ng_web_test_suite( "//tensorboard/webapp/metrics:integration_test", "//tensorboard/webapp/metrics:test_lib", "//tensorboard/webapp/metrics:utils_test", + "//tensorboard/webapp/metrics/data_source:card_interactions_data_source_test", "//tensorboard/webapp/metrics/data_source:metrics_data_source_test", "//tensorboard/webapp/metrics/effects:effects_test", "//tensorboard/webapp/metrics/store:store_test", @@ -358,6 +359,7 @@ tf_svg_bundle( "@com_google_material_design_icon//:notifications_none_24px.svg", "@com_google_material_design_icon//:palette_24px.svg", "@com_google_material_design_icon//:refresh_24px.svg", + "@com_google_material_design_icon//:science_24px.svg", "@com_google_material_design_icon//:search_24px.svg", "@com_google_material_design_icon//:settings_24px.svg", "@com_google_material_design_icon//:settings_backup_restore_24px.svg", diff --git a/tensorboard/webapp/metrics/BUILD b/tensorboard/webapp/metrics/BUILD index b8564af794..430a1d0208 100644 --- a/tensorboard/webapp/metrics/BUILD +++ b/tensorboard/webapp/metrics/BUILD @@ -15,6 +15,7 @@ tf_ng_module( "//tensorboard/webapp/core", "//tensorboard/webapp/metrics/actions", "//tensorboard/webapp/metrics/data_source", + "//tensorboard/webapp/metrics/data_source:card_interactions_data_source", "//tensorboard/webapp/metrics/effects", "//tensorboard/webapp/metrics/store", "//tensorboard/webapp/metrics/store:metrics_initial_state_provider", diff --git a/tensorboard/webapp/metrics/actions/index.ts b/tensorboard/webapp/metrics/actions/index.ts index a7f8f52d10..0645a385ac 100644 --- a/tensorboard/webapp/metrics/actions/index.ts +++ b/tensorboard/webapp/metrics/actions/index.ts @@ -23,13 +23,12 @@ import { TimeSeriesRequest, TimeSeriesResponse, } from '../data_source'; -import {CardState} from '../store/metrics_types'; +import {CardInteractions, CardState} from '../store/metrics_types'; import { CardId, HeaderEditInfo, HeaderToggleInfo, HistogramMode, - MinMaxStep, PluginType, TooltipSort, XAxisType, @@ -272,5 +271,15 @@ export const metricsHideEmptyCardsToggled = createAction( '[Metrics] Hide Empty Cards Changed' ); +export const metricsPreviousCardInteractionsChanged = createAction( + '[Metrics] Card Interactions Changed', + props<{cardInteractions: CardInteractions}>() +); + +export const metricsCardClicked = createAction( + '[Metrics] Card Clicked', + props<{cardId: string}>() +); + // TODO(jieweiwu): Delete after internal code is updated. export const stepSelectorTimeSelectionChanged = timeSelectionChanged; diff --git a/tensorboard/webapp/metrics/data_source/BUILD b/tensorboard/webapp/metrics/data_source/BUILD index b65f74c320..441ee5a39b 100644 --- a/tensorboard/webapp/metrics/data_source/BUILD +++ b/tensorboard/webapp/metrics/data_source/BUILD @@ -25,6 +25,18 @@ tf_ng_module( ], ) +tf_ng_module( + name = "card_interactions_data_source", + srcs = [ + "card_interactions_data_source.ts", + "card_interactions_data_source_module.ts", + ], + deps = [ + "//tensorboard/webapp/metrics/store:types", + "@npm//@angular/core", + ], +) + tf_ts_library( name = "types", srcs = [ @@ -70,3 +82,17 @@ tf_ts_library( "@npm//rxjs", ], ) + +tf_ts_library( + name = "card_interactions_data_source_test", + testonly = True, + srcs = [ + "card_interactions_data_source_test.ts", + ], + deps = [ + ":card_interactions_data_source", + "//tensorboard/webapp/angular:expect_angular_core_testing", + "//tensorboard/webapp/metrics:internal_types", + "@npm//@types/jasmine", + ], +) diff --git a/tensorboard/webapp/metrics/data_source/card_interactions_data_source.ts b/tensorboard/webapp/metrics/data_source/card_interactions_data_source.ts new file mode 100644 index 0000000000..c88ca44c0b --- /dev/null +++ b/tensorboard/webapp/metrics/data_source/card_interactions_data_source.ts @@ -0,0 +1,43 @@ +import {Injectable} from '@angular/core'; +import {CardInteractions} from '../store/metrics_types'; + +const CARD_INTERACTIONS_KEY = 'tb-card-interactions'; + +const MAX_RECORDS: Record = { + pins: 10, + clicks: 10, + tagFilters: 10, +}; + +@Injectable() +export class CardInteractionsDataSource { + saveCardInteractions(cardInteractions: CardInteractions) { + const trimmedInteractions: CardInteractions = { + pins: cardInteractions.pins.slice( + cardInteractions.pins.length - MAX_RECORDS.pins + ), + clicks: cardInteractions.clicks.slice( + cardInteractions.clicks.length - MAX_RECORDS.clicks + ), + tagFilters: cardInteractions.tagFilters.slice( + cardInteractions.tagFilters.length - MAX_RECORDS.tagFilters + ), + }; + localStorage.setItem( + CARD_INTERACTIONS_KEY, + JSON.stringify(trimmedInteractions) + ); + } + + getCardInteractions(): CardInteractions { + const existingInteractions = localStorage.getItem(CARD_INTERACTIONS_KEY); + if (existingInteractions) { + return JSON.parse(existingInteractions) as CardInteractions; + } + return { + tagFilters: [], + pins: [], + clicks: [], + }; + } +} diff --git a/tensorboard/webapp/metrics/data_source/card_interactions_data_source_module.ts b/tensorboard/webapp/metrics/data_source/card_interactions_data_source_module.ts new file mode 100644 index 0000000000..b7e04d45ec --- /dev/null +++ b/tensorboard/webapp/metrics/data_source/card_interactions_data_source_module.ts @@ -0,0 +1,22 @@ +/* Copyright 2023 The TensorFlow Authors. All Rights Reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +==============================================================================*/ +import {NgModule} from '@angular/core'; +import {CardInteractionsDataSource} from './card_interactions_data_source'; + +@NgModule({ + imports: [], + providers: [CardInteractionsDataSource], +}) +export class MetricsCardInteractionsDataSourceModule {} diff --git a/tensorboard/webapp/metrics/data_source/card_interactions_data_source_test.ts b/tensorboard/webapp/metrics/data_source/card_interactions_data_source_test.ts new file mode 100644 index 0000000000..5966f9dc62 --- /dev/null +++ b/tensorboard/webapp/metrics/data_source/card_interactions_data_source_test.ts @@ -0,0 +1,125 @@ +/* Copyright 2023 The TensorFlow Authors. All Rights Reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +==============================================================================*/ +import {TestBed} from '@angular/core/testing'; +import {CardInteractionsDataSource} from './card_interactions_data_source'; +import {PluginType} from '../internal_types'; + +describe('CardInteractionsDataSource Test', () => { + let mockStorage: Record; + let dataSource: CardInteractionsDataSource; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + providers: [CardInteractionsDataSource], + }); + + dataSource = TestBed.inject(CardInteractionsDataSource); + + mockStorage = {}; + spyOn(window.localStorage, 'setItem').and.callFake( + (key: string, value: string) => { + if (key !== 'tb-card-interactions') { + throw new Error('incorrect key used'); + } + + mockStorage[key] = value; + } + ); + + spyOn(window.localStorage, 'getItem').and.callFake((key: string) => { + if (key !== 'tb-card-interactions') { + throw new Error('incorrect key used'); + } + + return mockStorage[key]; + }); + }); + + describe('saveCardInteractions', () => { + it('only saves 10 pins', () => { + dataSource.saveCardInteractions({ + clicks: [], + tagFilters: [], + pins: Array.from({length: 12}).map((_, index) => ({ + cardId: `card-${index}`, + runId: null, + tag: 'foo', + plugin: PluginType.SCALARS, + })), + }); + + expect(dataSource.getCardInteractions().pins.length).toEqual(10); + }); + + it('only saves 10 clicks', () => { + dataSource.saveCardInteractions({ + pins: [], + tagFilters: [], + clicks: Array.from({length: 12}).map((_, index) => ({ + cardId: `card-${index}`, + runId: null, + tag: 'foo', + plugin: PluginType.SCALARS, + })), + }); + + expect(dataSource.getCardInteractions().clicks.length).toEqual(10); + }); + + it('only saves 10 tagFilgers', () => { + dataSource.saveCardInteractions({ + clicks: [], + tagFilters: Array.from({length: 12}).map((_, index) => + index.toString() + ), + pins: [], + }); + + expect(dataSource.getCardInteractions().tagFilters.length).toEqual(10); + }); + }); + + describe('getCardInteractions', () => { + it('returns all default state when key is not set', () => { + expect(dataSource.getCardInteractions()).toEqual({ + tagFilters: [], + pins: [], + clicks: [], + }); + }); + + it('returns previously written value', () => { + dataSource.saveCardInteractions({ + tagFilters: ['foo'], + clicks: [ + {cardId: '1', runId: null, tag: 'foo', plugin: PluginType.SCALARS}, + ], + pins: [ + {cardId: '2', runId: null, tag: 'bar', plugin: PluginType.SCALARS}, + ], + }); + + expect(dataSource.getCardInteractions()).toEqual({ + tagFilters: ['foo'], + clicks: [ + {cardId: '1', runId: null, tag: 'foo', plugin: PluginType.SCALARS}, + ], + pins: [ + {cardId: '2', runId: null, tag: 'bar', plugin: PluginType.SCALARS}, + ], + }); + }); + }); +}); diff --git a/tensorboard/webapp/metrics/effects/BUILD b/tensorboard/webapp/metrics/effects/BUILD index b0979fc828..a8d6602bdb 100644 --- a/tensorboard/webapp/metrics/effects/BUILD +++ b/tensorboard/webapp/metrics/effects/BUILD @@ -4,17 +4,22 @@ package(default_visibility = ["//tensorboard:internal"]) tf_ng_module( name = "effects", - srcs = ["index.ts"], + srcs = [ + "card_interaction_effects.ts", + "index.ts", + ], deps = [ "//tensorboard/webapp:app_state", "//tensorboard/webapp:selectors", "//tensorboard/webapp/app_routing:types", "//tensorboard/webapp/app_routing/actions", + "//tensorboard/webapp/app_routing/store", "//tensorboard/webapp/core/actions", "//tensorboard/webapp/core/store", "//tensorboard/webapp/metrics:types", "//tensorboard/webapp/metrics/actions", "//tensorboard/webapp/metrics/data_source", + "//tensorboard/webapp/metrics/data_source:card_interactions_data_source", "//tensorboard/webapp/metrics/store", "//tensorboard/webapp/types", "@npm//@angular/core", @@ -27,7 +32,10 @@ tf_ng_module( tf_ts_library( name = "effects_test", testonly = True, - srcs = ["metrics_effects_test.ts"], + srcs = [ + "card_interactions_effects_test.ts", + "metrics_effects_test.ts", + ], deps = [ ":effects", "//tensorboard/webapp:app_state", @@ -37,14 +45,18 @@ tf_ts_library( "//tensorboard/webapp/app_routing:testing", "//tensorboard/webapp/app_routing:types", "//tensorboard/webapp/app_routing/actions", + "//tensorboard/webapp/app_routing/store", "//tensorboard/webapp/core/actions", "//tensorboard/webapp/core/store", "//tensorboard/webapp/core/testing", + "//tensorboard/webapp/metrics:internal_types", "//tensorboard/webapp/metrics:test_lib", "//tensorboard/webapp/metrics:types", "//tensorboard/webapp/metrics/actions", "//tensorboard/webapp/metrics/data_source", + "//tensorboard/webapp/metrics/data_source:card_interactions_data_source", "//tensorboard/webapp/metrics/store", + "//tensorboard/webapp/testing:utils", "//tensorboard/webapp/types", "//tensorboard/webapp/util:dom", "//tensorboard/webapp/webapp_data_source:http_client_testing", diff --git a/tensorboard/webapp/metrics/effects/card_interaction_effects.ts b/tensorboard/webapp/metrics/effects/card_interaction_effects.ts new file mode 100644 index 0000000000..e862ffa01d --- /dev/null +++ b/tensorboard/webapp/metrics/effects/card_interaction_effects.ts @@ -0,0 +1,118 @@ +import {Injectable} from '@angular/core'; +import {Action, Store, createAction} from '@ngrx/store'; +import { + State, + getCardInteractions, + getCardMetadataMap, + getPreviousCardInteractions, +} from '../store'; +import {Actions, OnInitEffects, createEffect, ofType} from '@ngrx/effects'; +import {CardInteractionsDataSource} from '../data_source/card_interactions_data_source'; +import {withLatestFrom, skip, tap} from 'rxjs'; +import {metricsPreviousCardInteractionsChanged} from '../actions'; +import {getActiveNamespaceId} from '../../app_routing/store/app_routing_selectors'; +import {CardIdWithMetadata} from '../types'; + +const initAction = createAction('[Card Interaction Effects] Init'); + +@Injectable() +export class CardInteractionEffects implements OnInitEffects { + constructor( + private readonly actions$: Actions, + private readonly store: Store, + private readonly dataSource: CardInteractionsDataSource + ) {} + + /** @export */ + ngrxOnInitEffects(): Action { + return initAction(); + } + + private getCardInteractions$ = this.store.select(getCardInteractions).pipe( + // Don't get the initial state + skip(1) + ); + + private getPreviousCardInteractions$ = this.store + .select(getPreviousCardInteractions) + .pipe( + // Don't get the initial state + skip(1) + ); + + readonly onInitEffect$ = createEffect( + () => { + return this.actions$.pipe( + ofType(initAction), + tap(() => { + this.store.dispatch( + metricsPreviousCardInteractionsChanged({ + cardInteractions: this.dataSource.getCardInteractions(), + }) + ); + }) + ); + }, + {dispatch: false} + ); + + /** @export */ + readonly cardInteractionsEffect$ = createEffect( + () => { + return this.getCardInteractions$.pipe( + tap((cardInteractions) => { + this.dataSource.saveCardInteractions(cardInteractions); + }) + ); + }, + {dispatch: false} + ); + + /** @export */ + readonly updateInteractionsOnNavigationEffect$ = createEffect( + () => { + return this.store.select(getActiveNamespaceId).pipe( + withLatestFrom( + this.getCardInteractions$, + this.getPreviousCardInteractions$, + this.store.select(getCardMetadataMap) + ), + tap(([, cardInteractions, previousCardInteractions, metadataMap]) => { + const nextCardInteractions = { + pins: makeUnique([ + ...cardInteractions.pins, + ...previousCardInteractions.pins, + ]), + clicks: makeUnique([ + ...cardInteractions.clicks, + ...previousCardInteractions.clicks, + ]), + tagFilters: Array.from( + new Set([ + ...cardInteractions.tagFilters, + ...previousCardInteractions.tagFilters, + ]) + ), + }; + + this.store.dispatch( + metricsPreviousCardInteractionsChanged({ + cardInteractions: nextCardInteractions, + }) + ); + + function makeUnique(cardMetadata: CardIdWithMetadata[]) { + return Array.from( + new Set(cardMetadata.map(({cardId}) => cardId)) + ).map((cardId) => ({...metadataMap[cardId], cardId})); + } + }) + ); + }, + {dispatch: false} + ); +} + +export const TEST_ONLY = { + initAction, +}; diff --git a/tensorboard/webapp/metrics/effects/card_interactions_effects_test.ts b/tensorboard/webapp/metrics/effects/card_interactions_effects_test.ts new file mode 100644 index 0000000000..754624b185 --- /dev/null +++ b/tensorboard/webapp/metrics/effects/card_interactions_effects_test.ts @@ -0,0 +1,223 @@ +import {Action, Store} from '@ngrx/store'; +import {Subject} from 'rxjs'; +import {CardInteractionsDataSource} from '../data_source/card_interactions_data_source'; +import {MockStore} from '@ngrx/store/testing'; +import {State} from '../../app_state'; +import {CardInteractionEffects, TEST_ONLY} from './card_interaction_effects'; +import {TestBed, fakeAsync} from '@angular/core/testing'; +import {provideMockTbStore} from '../../testing/utils'; +import {provideMockActions} from '@ngrx/effects/testing'; +import * as actions from '../actions'; +import {CardIdWithMetadata, PluginType} from '../internal_types'; +import { + getCardInteractions, + getCardMetadataMap, + getPreviousCardInteractions, +} from '../store'; +import {getActiveNamespaceId} from '../../app_routing/store/app_routing_selectors'; + +describe('CardInteractions Effects', () => { + let dataSource: CardInteractionsDataSource; + let effects: CardInteractionEffects; + let store: MockStore; + let actions$: Subject; + let mockStorage: Record; + let dispatchedActions: Action[]; + + beforeEach(async () => { + actions$ = new Subject(); + mockStorage = {}; + spyOn(window.localStorage, 'setItem').and.callFake( + (key: string, value: string) => { + mockStorage[key] = value; + } + ); + + spyOn(window.localStorage, 'getItem').and.callFake((key: string) => { + return mockStorage[key]; + }); + + await TestBed.configureTestingModule({ + imports: [], + providers: [ + provideMockActions(actions$), + CardInteractionsDataSource, + CardInteractionEffects, + provideMockTbStore(), + ], + }).compileComponents(); + + store = TestBed.inject>(Store) as MockStore; + dataSource = TestBed.inject(CardInteractionsDataSource); + effects = TestBed.inject(CardInteractionEffects); + + dispatchedActions = []; + spyOn(store, 'dispatch').and.callFake((action: Action) => { + dispatchedActions.push(action); + }); + }); + + afterEach(() => { + store.resetSelectors(); + }); + + describe('onInitEffect$', () => { + it('rehydrates previousCardInteractions from data source', () => { + const previousCardInteractions = { + tagFilters: ['foo', 'bar'], + pins: [ + { + cardId: 'card1', + runId: null, + plugin: PluginType.SCALARS, + tag: 'tagA', + }, + ], + clicks: [ + { + cardId: 'card2', + runId: null, + plugin: PluginType.SCALARS, + tag: 'tagB', + }, + ], + }; + dataSource.saveCardInteractions(previousCardInteractions); + + effects.onInitEffect$.subscribe(); + actions$.next(TEST_ONLY.initAction()); + expect(dispatchedActions).toEqual([ + actions.metricsPreviousCardInteractionsChanged({ + cardInteractions: previousCardInteractions, + }), + ]); + }); + }); + + describe('cardInteractionsEffect$', () => { + it('skips the initial state', fakeAsync(() => { + const saveSpy = spyOn( + dataSource, + 'saveCardInteractions' + ).and.callThrough(); + effects.cardInteractionsEffect$.subscribe(); + expect(saveSpy).not.toHaveBeenCalled(); + + store.overrideSelector(getCardInteractions, { + tagFilters: ['foo', 'bar'], + pins: [ + { + cardId: 'card1', + runId: null, + plugin: PluginType.SCALARS, + tag: 'tagA', + }, + ], + clicks: [ + { + cardId: 'card2', + runId: null, + plugin: PluginType.SCALARS, + tag: 'tagB', + }, + ], + }); + store.refreshState(); + + expect(saveSpy).toHaveBeenCalled(); + })); + }); + + describe('updateInteractionsOnNavigationEffect$', () => { + let card1a: CardIdWithMetadata; + let card1b: CardIdWithMetadata; + let card2a: CardIdWithMetadata; + let card2b: CardIdWithMetadata; + + beforeEach(() => { + card1a = { + cardId: 'card1a', + runId: null, + tag: '1a', + plugin: PluginType.SCALARS, + }; + card1b = { + cardId: 'card1b', + runId: null, + tag: '1b', + plugin: PluginType.SCALARS, + }; + card2a = { + cardId: 'card2a', + runId: null, + tag: '2a', + plugin: PluginType.SCALARS, + }; + card2b = { + cardId: 'card2b', + runId: null, + tag: '2b', + plugin: PluginType.SCALARS, + }; + + store.overrideSelector(getCardMetadataMap, { + card1a, + card1b, + card2a, + card2b, + }); + + effects.updateInteractionsOnNavigationEffect$.subscribe(); + }); + + it('merges current and previous card interactions', () => { + store.overrideSelector(getCardInteractions, { + pins: [card1a], + clicks: [card1b], + tagFilters: ['foo'], + }); + store.overrideSelector(getPreviousCardInteractions, { + pins: [card2a], + clicks: [card2b], + tagFilters: ['bar'], + }); + store.overrideSelector(getActiveNamespaceId, 'namespace1'); + store.refreshState(); + + expect(dispatchedActions).toEqual([ + actions.metricsPreviousCardInteractionsChanged({ + cardInteractions: { + pins: [card1a, card2a], + clicks: [card1b, card2b], + tagFilters: ['foo', 'bar'], + }, + }), + ]); + }); + + it('does not emit duplicate cardIds', () => { + store.overrideSelector(getCardInteractions, { + pins: [card1a], + clicks: [card1b], + tagFilters: ['foo'], + }); + store.overrideSelector(getPreviousCardInteractions, { + pins: [card1a], + clicks: [card1b], + tagFilters: ['foo'], + }); + store.overrideSelector(getActiveNamespaceId, 'namespace1'); + store.refreshState(); + + expect(dispatchedActions).toEqual([ + actions.metricsPreviousCardInteractionsChanged({ + cardInteractions: { + pins: [card1a], + clicks: [card1b], + tagFilters: ['foo'], + }, + }), + ]); + }); + }); +}); diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index 8e3929a636..0a8c02241a 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -399,6 +399,16 @@ const {initialState, reducers: namespaceContextedReducer} = settings: METRICS_SETTINGS_DEFAULT, settingOverrides: {}, visibleCardMap: new Map(), + previousCardInteractions: { + tagFilters: [], + pins: [], + clicks: [], + }, + cardInteractions: { + tagFilters: [], + pins: [], + clicks: [], + }, }, /** onNavigated */ @@ -731,9 +741,39 @@ const reducer = createReducer( }; }), on(actions.metricsTagFilterChanged, (state, {tagFilter}) => { + const nextCardInteractions = { + ...state.cardInteractions, + }; + const previousTagFilter = state.tagFilter; + // Ensuring we do not create redundant tag filter as the user types + if (tagFilter === '') { + nextCardInteractions.tagFilters = nextCardInteractions.tagFilters.slice( + 0, + nextCardInteractions.tagFilters.length - 1 + ); + } else if ( + previousTagFilter === '' || + !nextCardInteractions.tagFilters.length + ) { + nextCardInteractions.tagFilters = [ + ...nextCardInteractions.tagFilters, + tagFilter, + ]; + } else if ( + tagFilter.includes(previousTagFilter) || + previousTagFilter.includes(tagFilter) + ) { + nextCardInteractions.tagFilters = nextCardInteractions.tagFilters.slice( + 0, + nextCardInteractions.tagFilters.length - 1 + ); + nextCardInteractions.tagFilters.push(tagFilter); + } + return { ...state, tagFilter, + cardInteractions: nextCardInteractions, }; }), on(actions.metricsChangeTooltipSort, (state, {sort}) => { @@ -1087,6 +1127,8 @@ const reducer = createReducer( let nextCardMetadataMap = {...state.cardMetadataMap}; let nextCardStepIndexMap = {...state.cardStepIndex}; let nextCardStateMap = {...state.cardStateMap}; + const nextCardInteractions = {...state.cardInteractions}; + const nextPreviousCardInteractions = {...state.previousCardInteractions}; if (isPinnedCopy) { const originalCardId = state.pinnedCardToOriginal.get(cardId); @@ -1096,6 +1138,13 @@ const reducer = createReducer( delete nextCardMetadataMap[cardId]; delete nextCardStepIndexMap[cardId]; delete nextCardStateMap[cardId]; + nextCardInteractions.pins = nextCardInteractions.pins.filter( + (cardMetadata) => originalCardId !== cardMetadata.cardId + ); + nextPreviousCardInteractions.pins = + nextPreviousCardInteractions.pins.filter( + (cardMetadata) => originalCardId !== cardMetadata.cardId + ); } else { if (shouldPin) { const resolvedResult = buildOrReturnStateWithPinnedCopy( @@ -1113,6 +1162,16 @@ const reducer = createReducer( nextCardMetadataMap = resolvedResult.cardMetadataMap; nextCardStepIndexMap = resolvedResult.cardStepIndex; nextCardStateMap = resolvedResult.cardStateMap; + if ( + !nextCardInteractions.pins.find( + (metadata) => cardId === metadata.cardId + ) + ) { + nextCardInteractions.pins = [ + ...nextCardInteractions.pins, + {...nextCardMetadataMap[cardId], cardId}, + ]; + } } else { const pinnedCardId = state.cardToPinnedCopy.get(cardId)!; nextCardToPinnedCopy.delete(cardId); @@ -1131,6 +1190,8 @@ const reducer = createReducer( cardToPinnedCopy: nextCardToPinnedCopy, cardToPinnedCopyCache: nextCardToPinnedCopyCache, pinnedCardToOriginal: nextPinnedCardToOriginal, + cardInteractions: nextCardInteractions, + previousCardInteractions: nextPreviousCardInteractions, }; }), on(actions.linkedTimeToggled, (state) => { @@ -1468,6 +1529,31 @@ const reducer = createReducer( }), on(actions.metricsSlideoutMenuClosed, (state) => { return {...state, isSlideoutMenuOpen: false}; + }), + on( + actions.metricsPreviousCardInteractionsChanged, + (state, {cardInteractions}) => { + return {...state, previousCardInteractions: cardInteractions}; + } + ), + on(actions.metricsCardClicked, (state, {cardId}) => { + const nextClicksIds = new Set( + state.cardInteractions.clicks.map(({cardId}) => cardId) + ); + nextClicksIds.add(cardId); + + const nextCardInteractions = { + ...state.cardInteractions, + clicks: Array.from(nextClicksIds).map((cardId) => ({ + ...state.cardMetadataMap[cardId], + cardId, + })), + }; + + return { + ...state, + cardInteractions: nextCardInteractions, + }; }) ); diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index 55f927ccd7..0b344733c9 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -2884,6 +2884,23 @@ describe('metrics reducers', () => { cardToPinnedCopyCache: new Map([['card1', expectedPinnedCopyId]]), pinnedCardToOriginal: new Map([[expectedPinnedCopyId, 'card1']]), timeSeriesData, + cardInteractions: { + pins: [ + { + cardId: 'card1', + plugin: PluginType.SCALARS, + runId: null, + tag: 'tagA', + }, + ], + clicks: [], + tagFilters: [], + }, + previousCardInteractions: { + pins: [], + clicks: [], + tagFilters: [], + }, }); expect(nextState).toEqual(expectedState); }); diff --git a/tensorboard/webapp/metrics/store/metrics_selectors.ts b/tensorboard/webapp/metrics/store/metrics_selectors.ts index 0e35780e6d..cc80689419 100644 --- a/tensorboard/webapp/metrics/store/metrics_selectors.ts +++ b/tensorboard/webapp/metrics/store/metrics_selectors.ts @@ -46,6 +46,7 @@ import { METRICS_FEATURE_KEY, RunToSeries, TagMetadata, + CardInteractions, } from './metrics_types'; import {ColumnHeader, DataTableMode} from '../../widgets/data_table/types'; import {Extent} from '../../widgets/line_chart_v2/lib/public_types'; @@ -130,7 +131,7 @@ export const getCardTimeSeries = createSelector( } ); -const getCardMetadataMap = createSelector( +export const getCardMetadataMap = createSelector( selectMetricsState, (state: MetricsState): CardMetadataMap => { return state.cardMetadataMap; @@ -254,6 +255,76 @@ export const getPinnedCardsWithMetadata = createSelector( } ); +export const getCardInteractions = createSelector( + selectMetricsState, + (state): CardInteractions => { + return state.cardInteractions; + } +); + +export const getPreviousCardInteractions = createSelector( + selectMetricsState, + (state): CardInteractions => { + return state.previousCardInteractions; + } +); + +const getSuggestedCardIds = createSelector( + getPreviousCardInteractions, + getCardToPinnedCopy, + getCardMetadataMap, + (cardInteractions, cardToPinnedCopy, cardMetadataMap): NonPinnedCardId[] => { + const previousPins = cardInteractions.pins + .map(({cardId}) => cardId) + .filter((cardId) => cardMetadataMap[cardId]) + .reverse(); + + const previousTagSearches = cardInteractions.tagFilters + .map((tagFilter) => { + return Object.entries(cardMetadataMap) + .filter(([, metadata]) => { + return metadata.tag.match(tagFilter); + }) + .map(([cardId]) => cardId); + }) + .flat() + .filter(Boolean) + .slice(-3) + .reverse(); + + const previouslyClickedCards = cardInteractions.clicks + .map(({cardId}) => cardId) + .filter((cardId) => cardMetadataMap[cardId]) + .slice(-3) + .reverse(); + + return Array.from( + new Set([ + ...previousPins, + ...previousTagSearches, + ...previouslyClickedCards, + ]) + ) + .filter((cardId) => !cardToPinnedCopy.get(cardId)) + .slice(0, 10); + } +); + +/** + * Returns an ordered list of cards that a user may be interested in. + */ +export const getSuggestedCardsWithMetadata = createSelector( + getSuggestedCardIds, + getCardMetadataMap, + (getSuggestedCardIds, metadataMap): DeepReadonly => { + return getSuggestedCardIds + .filter((cardId) => metadataMap.hasOwnProperty(cardId)) + .map((cardId) => { + return {cardId, ...metadataMap[cardId]}; + }); + } +); + /** * Returns true if a card is pinned or a separate card exists that is a pinned * copy of this card. Defaults to false if the card is unknown. diff --git a/tensorboard/webapp/metrics/store/metrics_selectors_test.ts b/tensorboard/webapp/metrics/store/metrics_selectors_test.ts index 0307d7f8bd..d829a4ff42 100644 --- a/tensorboard/webapp/metrics/store/metrics_selectors_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_selectors_test.ts @@ -25,7 +25,12 @@ import { createScalarStepData, createTimeSeriesData, } from '../testing'; -import {HistogramMode, TooltipSort, XAxisType} from '../types'; +import { + CardIdWithMetadata, + HistogramMode, + TooltipSort, + XAxisType, +} from '../types'; import { ColumnHeader, ColumnHeaderType, @@ -1004,6 +1009,87 @@ describe('metrics selectors', () => { }); }); + describe('getSuggestedCardsWithMetadata', () => { + let mockCards: CardIdWithMetadata[]; + let cardMetadataMap: Record; + + beforeEach(() => { + mockCards = Array.from({length: 15}).map((_, index) => ({ + cardId: `card${index}`, + tag: 'a', + runId: null, + plugin: PluginType.SCALARS, + })); + + cardMetadataMap = mockCards.reduce((map, card) => { + map[card.cardId] = card; + return map; + }, {} as Record); + }); + + it('does not return more than 3 cards based on previous tag searches', () => { + const state = appStateFromMetricsState( + buildMetricsState({ + cardMetadataMap, + previousCardInteractions: { + pins: [], + clicks: [], + tagFilters: ['a'], + }, + }) + ); + expect(selectors.getSuggestedCardsWithMetadata(state)).toEqual([ + mockCards[14], + mockCards[13], + mockCards[12], + ]); + }); + + it('does not return more than 3 previously clicked cards', () => { + const state = appStateFromMetricsState( + buildMetricsState({ + cardMetadataMap, + previousCardInteractions: { + pins: [], + clicks: mockCards, + tagFilters: [], + }, + }) + ); + expect(selectors.getSuggestedCardsWithMetadata(state)).toEqual([ + mockCards[14], + mockCards[13], + mockCards[12], + ]); + }); + + it('never returns more than 10 cards', () => { + const state = appStateFromMetricsState( + buildMetricsState({ + cardMetadataMap, + previousCardInteractions: { + pins: mockCards.slice(0, 5), + clicks: mockCards.slice(5, 10), + tagFilters: ['a'], + }, + }) + ); + + expect(selectors.getSuggestedCardsWithMetadata(state)).toEqual([ + mockCards[4], + mockCards[3], + mockCards[2], + mockCards[1], + mockCards[0], + mockCards[14], + mockCards[13], + mockCards[12], + mockCards[9], + mockCards[8], + ]); + }); + }); + describe('getCardPinnedState', () => { it('returns false if no card exists', () => { selectors.getCardPinnedState.release(); diff --git a/tensorboard/webapp/metrics/store/metrics_types.ts b/tensorboard/webapp/metrics/store/metrics_types.ts index b3364cc196..669a848ddf 100644 --- a/tensorboard/webapp/metrics/store/metrics_types.ts +++ b/tensorboard/webapp/metrics/store/metrics_types.ts @@ -27,6 +27,7 @@ import { } from '../data_source'; import { CardId, + CardIdWithMetadata, CardMetadata, CardUniqueInfo, HistogramMode, @@ -166,6 +167,12 @@ export type CardStepIndexMap = Record< CardStepIndexMetaData | null >; +export type CardInteractions = { + tagFilters: string[]; + pins: CardIdWithMetadata[]; + clicks: CardIdWithMetadata[]; +}; + export type CardToPinnedCard = Map; export type PinnedCardToCard = Map; @@ -254,6 +261,8 @@ export interface MetricsNonNamespacedState { * Map from ElementId to CardId. Only contains all visible cards. */ visibleCardMap: Map; + previousCardInteractions: CardInteractions; + cardInteractions: CardInteractions; } export type MetricsState = NamespaceContextedState< diff --git a/tensorboard/webapp/metrics/testing.ts b/tensorboard/webapp/metrics/testing.ts index fbb8a94f76..980c995243 100644 --- a/tensorboard/webapp/metrics/testing.ts +++ b/tensorboard/webapp/metrics/testing.ts @@ -112,6 +112,16 @@ function buildBlankState(): MetricsState { isSettingsPaneOpen: false, isSlideoutMenuOpen: false, tableEditorSelectedTab: DataTableMode.SINGLE, + cardInteractions: { + pins: [], + clicks: [], + tagFilters: [], + }, + previousCardInteractions: { + pins: [], + clicks: [], + tagFilters: [], + }, }; }