diff --git a/tensorboard/webapp/metrics/data_source/types.ts b/tensorboard/webapp/metrics/data_source/types.ts index 60730dfb00..9fb7769b70 100644 --- a/tensorboard/webapp/metrics/data_source/types.ts +++ b/tensorboard/webapp/metrics/data_source/types.ts @@ -52,6 +52,14 @@ export enum PluginType { IMAGES = 'images', } +export function isPluginType(text: string): text is PluginType { + return ( + text === PluginType.SCALARS || + text === PluginType.HISTOGRAMS || + text === PluginType.IMAGES + ); +} + export type SampledPluginType = PluginType.IMAGES; const sampledPluginTypes = [PluginType.IMAGES]; diff --git a/tensorboard/webapp/metrics/store/BUILD b/tensorboard/webapp/metrics/store/BUILD index c140afd780..d9b50f8b6d 100644 --- a/tensorboard/webapp/metrics/store/BUILD +++ b/tensorboard/webapp/metrics/store/BUILD @@ -14,6 +14,8 @@ tf_ts_library( ":types", "//tensorboard/webapp:app_state", "//tensorboard/webapp/app_routing:route_contexted_reducer_helper", + "//tensorboard/webapp/app_routing:types", + "//tensorboard/webapp/app_routing/actions", "//tensorboard/webapp/core/actions", "//tensorboard/webapp/metrics:types", "//tensorboard/webapp/metrics/actions", diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index 553313b02c..ea62f45105 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -16,7 +16,9 @@ import {Action, createReducer, on} from '@ngrx/store'; import * as coreActions from '../../core/actions'; import {DataLoadState} from '../../types/data'; +import {stateRehydratedFromUrl} from '../../app_routing/actions'; import {createRouteContextedState} from '../../app_routing/route_contexted_reducer_helper'; +import {RouteKind} from '../../app_routing/types'; import {mapObjectValues} from '../../util/lang'; import {composeReducers} from '../../util/ngrx'; import * as actions from '../actions'; @@ -33,13 +35,17 @@ import { } from '../data_source'; import { CardId, + CardUniqueInfo, CardMetadata, HistogramMode, TooltipSort, + URLDeserializedState, XAxisType, } from '../types'; import { + buildOrReturnStateWithUnresolvedImportedPins, + buildOrReturnStateWithPinnedCopy, createPluginDataWithLoadable, createRunToLoadState, getCardId, @@ -50,10 +56,12 @@ import { import { CardMetadataMap, CardStepIndexMap, + CardToPinnedCard, MetricsRoutefulState, MetricsRoutelessState, MetricsState, NonSampledPluginTagMetadata, + PinnedCardToCard, TagMetadata, TimeSeriesData, TimeSeriesLoadable, @@ -189,6 +197,19 @@ function buildResetLoadable(loadable: TimeSeriesLoadable) { return {...loadable, runToLoadState}; } +/** + * Returns an identifier useful for comparing a card in storage with a real card + * with loaded metadata. + */ +function serializeCardUniqueInfo( + plugin: string, + tag: string, + runId?: string | null, + sample?: number +): string { + return JSON.stringify([plugin, tag, runId || '', sample]); +} + const {initialState, reducers: routeContextReducer} = createRouteContextedState( { // Backend data. @@ -212,6 +233,7 @@ const {initialState, reducers: routeContextReducer} = createRouteContextedState( cardList: [], cardToPinnedCopy: new Map(), pinnedCardToOriginal: new Map(), + unresolvedImportedPinnedCards: [], cardMetadataMap: {}, cardStepIndex: {}, @@ -252,6 +274,61 @@ const {initialState, reducers: routeContextReducer} = createRouteContextedState( const reducer = createReducer( initialState, + on(stateRehydratedFromUrl, (state, {routeKind, partialState}) => { + if ( + routeKind !== RouteKind.EXPERIMENT && + routeKind !== RouteKind.COMPARE_EXPERIMENT + ) { + return state; + } + // The URL contains pinned cards + unresolved imported pins. Keep these sets + // mutually exclusive, and do not add duplicate any unresolved imported + // cards. + const serializedCardUniqueInfos = new Set(); + + // Visit existing pins. + for (const pinnedCardId of state.pinnedCardToOriginal.keys()) { + const {plugin, tag, runId, sample} = state.cardMetadataMap[pinnedCardId]; + serializedCardUniqueInfos.add( + serializeCardUniqueInfo(plugin, tag, runId, sample) + ); + } + + // We need to include previous unresolved imported pins, because the new + // hydrated state might not include them. For example, navigating from + // experiment A (with pins) --> B --> A, we want to ensure that rehydration + // does not drop the old unresolved pins on A. + const hydratedState = partialState as URLDeserializedState; + const unresolvedImportedPinnedCards = [] as CardUniqueInfo[]; + for (const card of [ + ...state.unresolvedImportedPinnedCards, + ...hydratedState.metrics.pinnedCards, + ]) { + const cardUniqueInfoString = serializeCardUniqueInfo( + card.plugin, + card.tag, + card.runId, + card.sample + ); + if (!serializedCardUniqueInfos.has(cardUniqueInfoString)) { + serializedCardUniqueInfos.add(cardUniqueInfoString); + unresolvedImportedPinnedCards.push(card); + } + } + + const resolvedResult = buildOrReturnStateWithUnresolvedImportedPins( + unresolvedImportedPinnedCards, + state.cardList, + state.cardMetadataMap, + state.cardToPinnedCopy, + state.pinnedCardToOriginal, + state.cardStepIndex + ); + return { + ...state, + ...resolvedResult, + }; + }), on(coreActions.reload, coreActions.manualReload, (state) => { const nextTagMetadataLoaded = state.tagMetadataLoaded === DataLoadState.LOADING @@ -312,23 +389,34 @@ const reducer = createReducer( // metadata does not include it. const nextCardMetadataMap = {...state.cardMetadataMap}; const nextCardMetadataList = buildCardMetadataList(nextTagMetadata); - const nextCardList = [...state.cardList]; + const newCardIds = []; // Create new cards for unseen metadata. for (const cardMetadata of nextCardMetadataList) { const cardId = getCardId(cardMetadata); if (!state.cardMetadataMap.hasOwnProperty(cardId)) { nextCardMetadataMap[cardId] = cardMetadata; - nextCardList.push(cardId); + newCardIds.push(cardId); } } + const nextCardList = [...state.cardList, ...newCardIds]; + + const resolvedResult = buildOrReturnStateWithUnresolvedImportedPins( + state.unresolvedImportedPinnedCards, + newCardIds, + nextCardMetadataMap, + state.cardToPinnedCopy, + state.pinnedCardToOriginal, + state.cardStepIndex + ); + return { ...state, + ...resolvedResult, tagMetadataLoaded: DataLoadState.LOADED, tagMetadata: nextTagMetadata, cardList: nextCardList, - cardMetadataMap: nextCardMetadataMap, }; } ), @@ -607,10 +695,10 @@ const reducer = createReducer( ? false : !state.cardToPinnedCopy.has(cardId); - const nextCardToPinnedCopy = new Map(state.cardToPinnedCopy); - const nextPinnedCardToOriginal = new Map(state.pinnedCardToOriginal); - const nextCardMetadataMap = {...state.cardMetadataMap}; - const nextCardStepIndexMap = {...state.cardStepIndex}; + let nextCardToPinnedCopy = new Map(state.cardToPinnedCopy); + let nextPinnedCardToOriginal = new Map(state.pinnedCardToOriginal); + let nextCardMetadataMap = {...state.cardMetadataMap}; + let nextCardStepIndexMap = {...state.cardStepIndex}; if (isPinnedCopy) { const originalCardId = state.pinnedCardToOriginal.get(cardId); @@ -620,19 +708,17 @@ const reducer = createReducer( delete nextCardStepIndexMap[cardId]; } else { if (shouldPin) { - // Create a pinned copy. Copies step index from the original card. - const pinnedCardId = getPinnedCardId(cardId); - nextCardToPinnedCopy.set(cardId, pinnedCardId); - nextPinnedCardToOriginal.set(pinnedCardId, cardId); - if (state.cardStepIndex.hasOwnProperty(cardId)) { - nextCardStepIndexMap[pinnedCardId] = state.cardStepIndex[cardId]; - } - - const metadata = state.cardMetadataMap[cardId]; - if (!metadata) { - throw new Error('Cannot pin a card without metadata'); - } - nextCardMetadataMap[pinnedCardId] = metadata; + const resolvedResult = buildOrReturnStateWithPinnedCopy( + cardId, + nextCardToPinnedCopy, + nextPinnedCardToOriginal, + nextCardStepIndexMap, + nextCardMetadataMap + ); + nextCardToPinnedCopy = resolvedResult.cardToPinnedCopy; + nextPinnedCardToOriginal = resolvedResult.pinnedCardToOriginal; + nextCardMetadataMap = resolvedResult.cardMetadataMap; + nextCardStepIndexMap = resolvedResult.cardStepIndex; } else { const pinnedCardId = state.cardToPinnedCopy.get(cardId)!; nextCardToPinnedCopy.delete(cardId); diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index bd61a3e530..1a20359e8c 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -30,6 +30,7 @@ import { buildMetricsState, buildTagMetadata, buildTimeSeriesData, + createCardMetadata, createHistogramStepData, createImageStepData, createScalarStepData, @@ -254,6 +255,113 @@ describe('metrics reducers', () => { ); }); + it('resolves imported pins by automatically creating pinned copies', () => { + const fakeCardMetadata = { + plugin: PluginType.SCALARS, + tag: 'tagA', + runId: null, + }; + const stepCount = 10; + const expectedCardId = getCardId(fakeCardMetadata); + const expectedPinnedCopyId = getPinnedCardId(expectedCardId); + const beforeState = buildMetricsState({ + cardMetadataMap: {}, + cardList: [], + cardStepIndex: { + [expectedCardId]: stepCount - 1, + }, + cardToPinnedCopy: new Map(), + pinnedCardToOriginal: new Map(), + unresolvedImportedPinnedCards: [ + {plugin: PluginType.SCALARS, tag: 'tagA'}, + {plugin: PluginType.SCALARS, tag: 'tagB'}, + ], + }); + const nextState = reducers( + beforeState, + actions.metricsTagMetadataLoaded({ + tagMetadata: { + ...buildDataSourceTagMetadata(), + [PluginType.SCALARS]: { + tagDescriptions: {}, + runTagInfo: {run1: ['tagA']}, + }, + }, + }) + ); + + const { + cardMetadataMap, + cardList, + cardStepIndex, + cardToPinnedCopy, + pinnedCardToOriginal, + unresolvedImportedPinnedCards, + } = nextState; + expect({ + cardMetadataMap, + cardList, + cardStepIndex, + cardToPinnedCopy, + pinnedCardToOriginal, + unresolvedImportedPinnedCards, + }).toEqual({ + cardMetadataMap: { + [expectedCardId]: fakeCardMetadata, + [expectedPinnedCopyId]: fakeCardMetadata, + }, + cardList: [expectedCardId], + cardStepIndex: { + [expectedCardId]: stepCount - 1, + [expectedPinnedCopyId]: stepCount - 1, + }, + cardToPinnedCopy: new Map([[expectedCardId, expectedPinnedCopyId]]), + pinnedCardToOriginal: new Map([[expectedPinnedCopyId, expectedCardId]]), + unresolvedImportedPinnedCards: [ + {plugin: PluginType.SCALARS, tag: 'tagB'}, + ], + }); + }); + + it('does not resolve mismatching imported pins', () => { + const beforeState = buildMetricsState({ + cardToPinnedCopy: new Map(), + pinnedCardToOriginal: new Map(), + unresolvedImportedPinnedCards: [ + {plugin: PluginType.IMAGES, tag: 'tagA', runId: 'run1', sample: 5}, + {plugin: PluginType.IMAGES, tag: 'tagB', runId: 'run1', sample: 5}, + ], + }); + const nextState = reducers( + beforeState, + actions.metricsTagMetadataLoaded({ + tagMetadata: { + ...buildDataSourceTagMetadata(), + [PluginType.IMAGES]: { + tagDescriptions: {}, + tagRunSampledInfo: { + tagA: { + // Matching run, but incorrect sample. + run1: {maxSamplesPerStep: 1}, + }, + tagB: { + // Matching tag, sample, but incorrect run. + run10: {maxSamplesPerStep: 10}, + }, + }, + }, + }, + }) + ); + + expect(nextState.cardToPinnedCopy).toEqual(new Map()); + expect(nextState.pinnedCardToOriginal).toEqual(new Map()); + expect(nextState.unresolvedImportedPinnedCards).toEqual([ + {plugin: PluginType.IMAGES, tag: 'tagA', runId: 'run1', sample: 5}, + {plugin: PluginType.IMAGES, tag: 'tagB', runId: 'run1', sample: 5}, + ]); + }); + it('does not drop existing data', () => { const beforeState = { ...buildMetricsState(), @@ -1313,4 +1421,166 @@ describe('metrics reducers', () => { expect(nextState.tagGroupExpanded).toEqual(new Map([['foo', true]])); }); }); + + describe('pinned card hydration', () => { + it('ignores RouteKind EXPERIMENTS', () => { + const beforeState = buildMetricsState({ + unresolvedImportedPinnedCards: [], + }); + const action = routingActions.stateRehydratedFromUrl({ + routeKind: RouteKind.EXPERIMENTS, + partialState: { + metrics: { + pinnedCards: [{plugin: PluginType.SCALARS, tag: 'accuracy'}], + }, + }, + }); + const nextState = reducers(beforeState, action); + + expect(nextState.unresolvedImportedPinnedCards).toEqual([]); + }); + + it('populates ngrx store with unresolved imported pins', () => { + const beforeState = buildMetricsState({ + unresolvedImportedPinnedCards: [], + }); + const action = routingActions.stateRehydratedFromUrl({ + routeKind: RouteKind.EXPERIMENT, + partialState: { + metrics: { + pinnedCards: [{plugin: PluginType.SCALARS, tag: 'accuracy'}], + }, + }, + }); + const nextState = reducers(beforeState, action); + + expect(nextState.unresolvedImportedPinnedCards).toEqual([ + {plugin: PluginType.SCALARS, tag: 'accuracy'}, + ]); + }); + + it('resolves imported pins', () => { + const fakeMetadata = { + ...createCardMetadata(PluginType.SCALARS), + tag: 'accuracy', + }; + const beforeState = buildMetricsState({ + cardList: ['card1'], + cardMetadataMap: { + card1: fakeMetadata, + }, + tagMetadataLoaded: DataLoadState.LOADED, + tagMetadata: { + ...buildTagMetadata(), + [PluginType.SCALARS]: { + tagDescriptions: {}, + tagToRuns: {accuracy: ['run1']}, + }, + }, + }); + const action = routingActions.stateRehydratedFromUrl({ + routeKind: RouteKind.EXPERIMENT, + partialState: { + metrics: { + pinnedCards: [{plugin: PluginType.SCALARS, tag: 'accuracy'}], + }, + }, + }); + const nextState = reducers(beforeState, action); + + const pinnedCopyId = getPinnedCardId('card1'); + expect(nextState.pinnedCardToOriginal).toEqual( + new Map([[pinnedCopyId, 'card1']]) + ); + expect(nextState.cardToPinnedCopy).toEqual( + new Map([['card1', pinnedCopyId]]) + ); + expect(nextState.unresolvedImportedPinnedCards).toEqual([]); + }); + + it('does not add resolved pins to the unresolved imported pins', () => { + const fakeMetadata = {...createCardMetadata(), tag: 'accuracy'}; + const beforeState = buildMetricsState({ + cardMetadataMap: { + 'card-pin1': fakeMetadata, + card1: fakeMetadata, + }, + pinnedCardToOriginal: new Map([['card-pin1', 'card1']]), + unresolvedImportedPinnedCards: [], + }); + const action = routingActions.stateRehydratedFromUrl({ + routeKind: RouteKind.EXPERIMENT, + partialState: { + metrics: { + pinnedCards: [{plugin: PluginType.SCALARS, tag: 'accuracy'}], + }, + }, + }); + const nextState = reducers(beforeState, action); + + expect(nextState.unresolvedImportedPinnedCards).toEqual([]); + }); + + it('does not create duplicate unresolved imported pins', () => { + const beforeState = buildMetricsState({ + unresolvedImportedPinnedCards: [ + {plugin: PluginType.SCALARS, tag: 'accuracy'}, + ], + }); + const action = routingActions.stateRehydratedFromUrl({ + routeKind: RouteKind.EXPERIMENT, + partialState: { + metrics: { + pinnedCards: [{plugin: PluginType.SCALARS, tag: 'accuracy'}], + }, + }, + }); + const nextState = reducers(beforeState, action); + + expect(nextState.unresolvedImportedPinnedCards).toEqual([ + {plugin: PluginType.SCALARS, tag: 'accuracy'}, + ]); + }); + + it('does not create duplicates if URL contained duplicates', () => { + const beforeState = buildMetricsState(); + const action = routingActions.stateRehydratedFromUrl({ + routeKind: RouteKind.EXPERIMENT, + partialState: { + metrics: { + pinnedCards: [ + {plugin: PluginType.SCALARS, tag: 'accuracyAgain'}, + {plugin: PluginType.SCALARS, tag: 'accuracyAgain'}, + ], + }, + }, + }); + const nextState = reducers(beforeState, action); + + expect(nextState.unresolvedImportedPinnedCards).toEqual([ + {plugin: PluginType.SCALARS, tag: 'accuracyAgain'}, + ]); + }); + + it('does not clear unresolved imported pins if hydration is empty', () => { + const beforeState = buildMetricsState({ + unresolvedImportedPinnedCards: [ + {plugin: PluginType.SCALARS, tag: 'accuracy'}, + ], + }); + const action = routingActions.stateRehydratedFromUrl({ + routeKind: RouteKind.EXPERIMENT, + partialState: { + metrics: { + pinnedCards: [], + }, + }, + }); + const nextState = reducers(beforeState, action); + + expect(nextState.unresolvedImportedPinnedCards).toEqual([ + {plugin: PluginType.SCALARS, tag: 'accuracy'}, + ]); + }); + }); }); diff --git a/tensorboard/webapp/metrics/store/metrics_store_internal_utils.ts b/tensorboard/webapp/metrics/store/metrics_store_internal_utils.ts index 7bf81c4020..db9f933fbc 100644 --- a/tensorboard/webapp/metrics/store/metrics_store_internal_utils.ts +++ b/tensorboard/webapp/metrics/store/metrics_store_internal_utils.ts @@ -19,15 +19,28 @@ limitations under the License. import {DataLoadState} from '../../types/data'; import {isSampledPlugin, PluginType, SampledPluginType} from '../data_source'; -import {CardId, CardMetadata} from '../types'; +import {CardId, CardMetadata, CardUniqueInfo, NonPinnedCardId} from '../types'; import { + CardMetadataMap, + CardStepIndexMap, + CardToPinnedCard, + MetricsState, + PinnedCardToCard, RunToLoadState, TagMetadata, TimeSeriesData, TimeSeriesLoadables, } from './metrics_types'; +type ResolvedPinPartialState = Pick< + MetricsState, + | 'cardMetadataMap' + | 'cardToPinnedCopy' + | 'pinnedCardToOriginal' + | 'cardStepIndex' +>; + /** * Returns the loadable information for a specific tag, containing its series * data and load state. Returns `null` when the requested tag has no initial @@ -153,3 +166,128 @@ export function getRunIds( const tagToRunIds = tagMetadata[plugin].tagToRuns; return tagToRunIds.hasOwnProperty(tag) ? tagToRunIds[tag] : []; } + +/** + * Returns whether the CardMetadata exactly matches the pinned card from + * storage. + */ +function cardMatchesCardUniqueInfo( + cardMetadata: CardMetadata, + cardUniqueInfo: CardUniqueInfo +): boolean { + const noRunId = !cardMetadata.runId && !cardUniqueInfo.runId; + return ( + cardMetadata.plugin === cardUniqueInfo.plugin && + cardMetadata.tag === cardUniqueInfo.tag && + cardMetadata.sample === cardUniqueInfo.sample && + (cardMetadata.runId === cardUniqueInfo.runId || noRunId) + ); +} + +/** + * Attempts to resolve the imported pins against the list of non-pinned cards + * provided. Returns the resulting state. + * + * Note: this assumes input has already been sanitized and validated. Untrusted + * data from URLs must be cleaned before being passed to the store. + */ +export function buildOrReturnStateWithUnresolvedImportedPins( + unresolvedImportedPinnedCards: CardUniqueInfo[], + nonPinnedCards: NonPinnedCardId[], + cardMetadataMap: CardMetadataMap, + cardToPinnedCopy: CardToPinnedCard, + pinnedCardToOriginal: PinnedCardToCard, + cardStepIndexMap: CardStepIndexMap +): ResolvedPinPartialState & {unresolvedImportedPinnedCards: CardUniqueInfo[]} { + const unresolvedPinSet = new Set(unresolvedImportedPinnedCards); + const nonPinnedCardsWithMatch = []; + for (const unresolvedPin of unresolvedImportedPinnedCards) { + for (const nonPinnedCardId of nonPinnedCards) { + const cardMetadata = cardMetadataMap[nonPinnedCardId]; + if (cardMatchesCardUniqueInfo(cardMetadata, unresolvedPin)) { + nonPinnedCardsWithMatch.push(nonPinnedCardId); + unresolvedPinSet.delete(unresolvedPin); + break; + } + } + } + + if (!nonPinnedCardsWithMatch.length) { + return { + unresolvedImportedPinnedCards, + cardMetadataMap, + cardToPinnedCopy, + pinnedCardToOriginal, + cardStepIndex: cardStepIndexMap, + }; + } + + let stateWithResolvedPins = { + cardToPinnedCopy, + pinnedCardToOriginal, + cardStepIndex: cardStepIndexMap, + cardMetadataMap, + }; + for (const cardToPin of nonPinnedCardsWithMatch) { + stateWithResolvedPins = buildOrReturnStateWithPinnedCopy( + cardToPin, + stateWithResolvedPins.cardToPinnedCopy, + stateWithResolvedPins.pinnedCardToOriginal, + stateWithResolvedPins.cardStepIndex, + stateWithResolvedPins.cardMetadataMap + ); + } + + return { + ...stateWithResolvedPins, + unresolvedImportedPinnedCards: [...unresolvedPinSet], + }; +} + +/** + * Return the state produced by creating a new pinned copy of the provided card. + * May throw if the card provided has no metadata. + */ +export function buildOrReturnStateWithPinnedCopy( + cardId: NonPinnedCardId, + cardToPinnedCopy: CardToPinnedCard, + pinnedCardToOriginal: PinnedCardToCard, + cardStepIndexMap: CardStepIndexMap, + cardMetadataMap: CardMetadataMap +): ResolvedPinPartialState { + // No-op if the card already has a pinned copy. + if (cardToPinnedCopy.has(cardId)) { + return { + cardToPinnedCopy, + pinnedCardToOriginal, + cardStepIndex: cardStepIndexMap, + cardMetadataMap, + }; + } + + const nextCardToPinnedCopy = new Map(cardToPinnedCopy); + const nextPinnedCardToOriginal = new Map(pinnedCardToOriginal); + const nextCardStepIndexMap = {...cardStepIndexMap}; + const nextCardMetadataMap = {...cardMetadataMap}; + + // Create a pinned copy. Copies step index from the original card. + const pinnedCardId = getPinnedCardId(cardId); + nextCardToPinnedCopy.set(cardId, pinnedCardId); + nextPinnedCardToOriginal.set(pinnedCardId, cardId); + if (cardStepIndexMap.hasOwnProperty(cardId)) { + nextCardStepIndexMap[pinnedCardId] = cardStepIndexMap[cardId]; + } + + const metadata = cardMetadataMap[cardId]; + if (!metadata) { + throw new Error('Cannot pin a card without metadata'); + } + nextCardMetadataMap[pinnedCardId] = metadata; + + return { + cardToPinnedCopy: nextCardToPinnedCopy, + pinnedCardToOriginal: nextPinnedCardToOriginal, + cardStepIndex: nextCardStepIndexMap, + cardMetadataMap: nextCardMetadataMap, + }; +} diff --git a/tensorboard/webapp/metrics/store/metrics_store_internal_utils_test.ts b/tensorboard/webapp/metrics/store/metrics_store_internal_utils_test.ts index 45566dfc11..ed2103e678 100644 --- a/tensorboard/webapp/metrics/store/metrics_store_internal_utils_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_store_internal_utils_test.ts @@ -15,12 +15,15 @@ limitations under the License. import {DataLoadState} from '../../types/data'; import {PluginType} from '../data_source'; -import {buildTagMetadata} from '../testing'; +import {buildTagMetadata, createCardMetadata} from '../testing'; import { + buildOrReturnStateWithPinnedCopy, + buildOrReturnStateWithUnresolvedImportedPins, createPluginDataWithLoadable, createRunToLoadState, getCardId, + getPinnedCardId, getRunIds, getTimeSeriesLoadable, } from './metrics_store_internal_utils'; @@ -343,4 +346,91 @@ describe('metrics store utils', () => { ).toEqual(['run1', 'run3']); }); }); + + describe('buildOrReturnStateWithUnresolvedImportedPins', () => { + it('resolves imported pins', () => { + const matchingInfo = {plugin: PluginType.SCALARS, tag: 'accuracy'}; + const nonMatchingInfo = {plugin: PluginType.SCALARS, tag: 'accuracy2'}; + const result = buildOrReturnStateWithUnresolvedImportedPins( + [matchingInfo, nonMatchingInfo], + ['card1'], + {card1: {plugin: PluginType.SCALARS, tag: 'accuracy', runId: null}}, + new Map(), + new Map(), + {card1: 2} + ); + + const pinnedCardId = getPinnedCardId('card1'); + expect(result.unresolvedImportedPinnedCards).toEqual([nonMatchingInfo]); + expect(result.cardToPinnedCopy).toEqual( + new Map([['card1', pinnedCardId]]) + ); + expect(result.pinnedCardToOriginal).toEqual( + new Map([[pinnedCardId, 'card1']]) + ); + }); + }); + + describe('buildOrReturnStateWithPinnedCopy', () => { + it('adds a pinned copy properly', () => { + const { + cardToPinnedCopy, + pinnedCardToOriginal, + cardStepIndex, + cardMetadataMap, + } = buildOrReturnStateWithPinnedCopy( + 'card1', + new Map(), + new Map(), + {card1: 2}, + {card1: createCardMetadata()} + ); + const pinnedCardId = getPinnedCardId('card1'); + + expect(cardToPinnedCopy).toEqual(new Map([['card1', pinnedCardId]])); + expect(pinnedCardToOriginal).toEqual(new Map([[pinnedCardId, 'card1']])); + expect(cardStepIndex).toEqual({ + card1: 2, + [pinnedCardId]: 2, + }); + expect(cardMetadataMap).toEqual({ + card1: createCardMetadata(), + [pinnedCardId]: createCardMetadata(), + }); + }); + + it('throws if the original card does not have metadata', () => { + expect(() => { + buildOrReturnStateWithPinnedCopy('card1', new Map(), new Map(), {}, {}); + }).toThrow(); + }); + + it('no-ops if the card already has a pinned copy', () => { + const cardToPinnedCopy = new Map([['card1', 'card-pin1']]); + const pinnedCardToOriginal = new Map([['card-pin1', 'card1']]); + const cardStepIndexMap = {}; + const cardMetadataMap = {card1: createCardMetadata()}; + const originals = { + cardToPinnedCopy: new Map(cardToPinnedCopy), + pinnedCardToOriginal: new Map(pinnedCardToOriginal), + cardStepIndexMap: {...cardStepIndexMap}, + cardMetadataMap: {...cardMetadataMap}, + }; + + const result = buildOrReturnStateWithPinnedCopy( + 'card1', + cardToPinnedCopy, + pinnedCardToOriginal, + cardStepIndexMap, + cardMetadataMap + ); + + expect(result.cardToPinnedCopy).toEqual(originals.cardToPinnedCopy); + expect(result.pinnedCardToOriginal).toEqual( + originals.pinnedCardToOriginal + ); + expect(result.cardStepIndex).toEqual(originals.cardStepIndexMap); + expect(result.cardMetadataMap).toEqual(originals.cardMetadataMap); + }); + }); }); diff --git a/tensorboard/webapp/metrics/store/metrics_types.ts b/tensorboard/webapp/metrics/store/metrics_types.ts index e5563cd7a0..79e4ced14b 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, + CardUniqueInfo, CardMetadata, HistogramMode, NonPinnedCardId, @@ -125,13 +126,27 @@ export type CardStepIndexMap = Record< number | null >; +export type CardToPinnedCard = Map; + +export type PinnedCardToCard = Map; + export interface MetricsRoutefulState { tagMetadataLoaded: DataLoadState; tagMetadata: TagMetadata; // A list of card ids in the main content area, excluding pinned copies. cardList: NonPinnedCardId[]; - cardToPinnedCopy: Map; - pinnedCardToOriginal: Map; + cardToPinnedCopy: CardToPinnedCard; + pinnedCardToOriginal: PinnedCardToCard; + /** + * Pinned cards imported from storage that do not yet have a corresponding + * card (e.g. tag metadata might not be loaded yet). Resolving an imported + * card requires comparing its CardUniqueInfo to a resolved card. After + * resolution, it is removed from this collection and added to the + * appropriate data structures (e.g. pinnedCardToOriginal). + * + * These may become stale if runs are deleted from the experiment. + */ + unresolvedImportedPinnedCards: CardUniqueInfo[]; cardMetadataMap: CardMetadataMap; cardStepIndex: CardStepIndexMap; tagFilter: string; diff --git a/tensorboard/webapp/metrics/testing.ts b/tensorboard/webapp/metrics/testing.ts index ce145e66eb..132e83fee5 100644 --- a/tensorboard/webapp/metrics/testing.ts +++ b/tensorboard/webapp/metrics/testing.ts @@ -83,6 +83,7 @@ function buildBlankState(): MetricsState { cardList: [], cardToPinnedCopy: new Map(), pinnedCardToOriginal: new Map(), + unresolvedImportedPinnedCards: [], cardMetadataMap: {}, cardStepIndex: {}, visibleCards: new Set(), diff --git a/tensorboard/webapp/metrics/types.ts b/tensorboard/webapp/metrics/types.ts index 593dab1360..cc8102e7f5 100644 --- a/tensorboard/webapp/metrics/types.ts +++ b/tensorboard/webapp/metrics/types.ts @@ -46,8 +46,34 @@ export type NonPinnedCardId = string; export type PinnedCardId = string; +/** + * A unique identifier to a specific card instance in the UI. This is an opaque + * ID, meaning that consumers should never peer into/parse it and never assume + * that it will always be a string. + */ export type CardId = NonPinnedCardId | PinnedCardId; export type CardIdWithMetadata = CardMetadata & { cardId: CardId; }; + +/** + * The most minimal representation of a card that uniquely identifies it across + * a browser session. This information may be persisted in storage, retrieved, + * and used to match against an existing card with the same metadata. + */ +export interface CardUniqueInfo { + plugin: string; + tag: string; + runId?: string; + sample?: number; +} + +/** + * The metrics-related state created by deserializing a URL. + */ +export interface URLDeserializedState { + metrics: { + pinnedCards: CardUniqueInfo[]; + }; +}