diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index f68df520a1..0fbee6deb9 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -1051,11 +1051,11 @@ const reducer = createReducer( // Updates cardStepIndex only when toggle to enable linked time. if (nextLinkedTimeEnabled) { - const {min} = state.stepMinMax; - const startStep = min === Infinity ? 0 : min; + const {max} = state.stepMinMax; + const endStep = max === -Infinity ? 0 : max; nextLinkedTimeSelection = state.linkedTimeSelection ?? { - start: {step: startStep}, - end: null, + start: null, + end: {step: endStep}, }; nextCardStepIndexMap = generateNextCardStepIndexFromLinkedTimeSelection( @@ -1066,7 +1066,7 @@ const reducer = createReducer( ); nextStepSelectorEnabled = nextLinkedTimeEnabled; - nextRangeSelectionEnabled = Boolean(nextLinkedTimeSelection.end); + nextRangeSelectionEnabled = nextLinkedTimeSelection.start !== null; } return { @@ -1111,17 +1111,17 @@ const reducer = createReducer( end: {step: state.stepMinMax.max}, }; } - if (!linkedTimeSelection.end) { + if (linkedTimeSelection.start === null) { linkedTimeSelection = { ...linkedTimeSelection, - end: {step: state.stepMinMax.max}, + start: {step: state.stepMinMax.min}, }; } } else { if (linkedTimeSelection) { linkedTimeSelection = { ...linkedTimeSelection, - end: null, + start: null, }; } } @@ -1135,25 +1135,25 @@ const reducer = createReducer( }), on(actions.timeSelectionChanged, (state, change) => { const {cardId, timeSelection} = change; - const nextStartStep = timeSelection.start.step; - const nextEndStep = timeSelection.end?.step; - const end = - nextEndStep === undefined + const nextStartStep = timeSelection.start?.step; + const nextEndStep = timeSelection.end.step; + const start = + nextStartStep === undefined ? null - : {step: nextStartStep > nextEndStep ? nextStartStep : nextEndStep}; + : {step: nextStartStep < nextEndStep ? nextStartStep : nextEndStep}; let nextRangeSelectionEnabled = state.rangeSelectionEnabled; if (state.linkedTimeEnabled) { - // If there is no endStep then current selection state is single. + // If there is no startStep then current selection state is single. // Otherwise selection state is range. - nextRangeSelectionEnabled = nextEndStep !== undefined; + nextRangeSelectionEnabled = nextStartStep !== undefined; } const nextTimeSelection = { - start: { - step: nextStartStep, + start, + end: { + step: nextEndStep, }, - end, }; const nextCardStepIndexMap = generateNextCardStepIndexFromLinkedTimeSelection( @@ -1169,7 +1169,7 @@ const reducer = createReducer( timeSelection: nextTimeSelection, stepSelectionOverride: CardFeatureOverride.OVERRIDE_AS_ENABLED, rangeSelectionOverride: - nextTimeSelection.end?.step === undefined + nextTimeSelection.start?.step === undefined ? CardFeatureOverride.OVERRIDE_AS_DISABLED : CardFeatureOverride.OVERRIDE_AS_ENABLED, }; diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index d4547f0bb6..477ca3bf6f 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -2310,8 +2310,8 @@ describe('metrics reducers', () => { cardStateMap: { card1: { timeSelection: { - start: {step: 5}, - end: null, + start: null, + end: {step: 5}, }, tableExpanded: true, }, @@ -2327,8 +2327,8 @@ describe('metrics reducers', () => { expect(nextState.cardStateMap).toEqual({ card1: { timeSelection: { - start: {step: 5}, - end: null, + start: null, + end: {step: 5}, }, tableExpanded: false, }, @@ -2963,7 +2963,7 @@ describe('metrics reducers', () => { }); }); - it('sets `end` to null from data when `endStep` is not present', () => { + it('sets `start` to null from data when `startStep` is not present', () => { const before = buildMetricsState({ linkedTimeSelection: null, stepMinMax: {min: 0, max: 100}, @@ -2972,17 +2972,17 @@ describe('metrics reducers', () => { const after = reducers( before, actions.timeSelectionChanged({ - timeSelection: {start: {step: 2}, end: null}, + timeSelection: {start: null, end: {step: 2}}, }) ); expect(after.linkedTimeSelection).toEqual({ - start: {step: 2}, - end: null, + start: null, + end: {step: 2}, }); }); - it('sets `end` when `endStep` is present', () => { + it('sets `start` when `startStep` is present', () => { const before = buildMetricsState({ linkedTimeSelection: null, stepMinMax: {min: 0, max: 100}, @@ -3004,7 +3004,7 @@ describe('metrics reducers', () => { }); }); - it('flips `end` to `start` if new start is greater than new end', () => { + it('flips `start` to `end` if new end is less than new start', () => { const beforeState = buildMetricsState({ linkedTimeSelection: null, stepMinMax: {min: 0, max: 100}, @@ -3021,12 +3021,12 @@ describe('metrics reducers', () => { ); expect(nextState.linkedTimeSelection).toEqual({ - start: {step: 150}, - end: {step: 150}, + start: {step: 0}, + end: {step: 0}, }); }); - it('sets `rangeSelectionEnabled` to true when `endStep` is present', () => { + it('sets `rangeSelectionEnabled` to true when `startStep` is present', () => { const beforeState = buildMetricsState({ rangeSelectionEnabled: false, linkedTimeEnabled: true, @@ -3045,7 +3045,7 @@ describe('metrics reducers', () => { expect(nextState.rangeSelectionEnabled).toEqual(true); }); - it('sets `rangeSelectionEnabled` to true when `endStep` is 0', () => { + it('sets `rangeSelectionEnabled` to true when `startStep` is 0', () => { const beforeState = buildMetricsState({ rangeSelectionEnabled: false, linkedTimeEnabled: true, @@ -3055,8 +3055,8 @@ describe('metrics reducers', () => { beforeState, actions.timeSelectionChanged({ timeSelection: { - start: {step: 2}, - end: {step: 0}, + start: {step: 0}, + end: {step: 2}, }, }) ); @@ -3064,7 +3064,7 @@ describe('metrics reducers', () => { expect(nextState.rangeSelectionEnabled).toEqual(true); }); - it('sets `rangeSelectionEnabled` to false when only sets `startStep`', () => { + it('sets `rangeSelectionEnabled` to false when only sets `endStep`', () => { const beforeState1 = buildMetricsState({ rangeSelectionEnabled: true, linkedTimeEnabled: true, @@ -3074,8 +3074,8 @@ describe('metrics reducers', () => { beforeState1, actions.timeSelectionChanged({ timeSelection: { - start: {step: 2}, - end: null, + start: null, + end: {step: 2}, }, }) ); @@ -3130,8 +3130,8 @@ describe('metrics reducers', () => { beforeState, actions.timeSelectionChanged({ timeSelection: { - start: {step: 10}, - end: null, + start: null, + end: {step: 10}, }, }) ); @@ -3170,8 +3170,8 @@ describe('metrics reducers', () => { beforeState, actions.timeSelectionChanged({ timeSelection: { - start: {step: 15}, - end: null, + start: null, + end: {step: 15}, }, }) ); @@ -3192,8 +3192,8 @@ describe('metrics reducers', () => { actions.timeSelectionChanged({ cardId: 'card2', timeSelection: { - start: {step: 1}, - end: null, + start: null, + end: {step: 1}, }, }) ); @@ -3202,8 +3202,8 @@ describe('metrics reducers', () => { card1: {}, card2: { timeSelection: { - start: {step: 1}, - end: null, + start: null, + end: {step: 1}, }, stepSelectionOverride: CardFeatureOverride.OVERRIDE_AS_ENABLED, rangeSelectionOverride: CardFeatureOverride.OVERRIDE_AS_DISABLED, @@ -3254,7 +3254,7 @@ describe('metrics reducers', () => { }); }); - it('enables card specific range selection if an end value is provided', () => { + it('enables card specific range selection if an start value is provided', () => { const state1 = buildMetricsState({ cardStateMap: { card1: {}, @@ -3361,23 +3361,23 @@ describe('metrics reducers', () => { }); }); - it('adds linkedTimeSelection.end when toggled on, if needed', () => { + it('adds linkedTimeSelection.start when toggled on, if needed', () => { const state1 = buildMetricsState({ linkedTimeSelection: { - start: {step: 100}, - end: null, + start: null, + end: {step: 100}, }, rangeSelectionEnabled: false, }); const state2 = reducers(state1, actions.rangeSelectionToggled({})); expect(state2.linkedTimeSelection).toEqual({ - start: {step: 100}, - end: {step: -Infinity}, + start: {step: Infinity}, + end: {step: 100}, }); }); - it('removes linkedTimeSelection.end when toggled off, if needed', () => { + it('removes linkedTimeSelection.start when toggled off, if needed', () => { const state1 = buildMetricsState({ linkedTimeSelection: { start: {step: 100}, @@ -3388,8 +3388,8 @@ describe('metrics reducers', () => { const state2 = reducers(state1, actions.rangeSelectionToggled({})); expect(state2.linkedTimeSelection).toEqual({ - start: {step: 100}, - end: null, + start: null, + end: {step: 1000}, }); }); @@ -3612,7 +3612,7 @@ describe('metrics reducers', () => { }, }, }, - linkedTimeSelection: {start: {step: 20}, end: null}, + linkedTimeSelection: {start: null, end: {step: 20}}, cardStepIndex: {[imageCardId]: buildStepIndexMetadata({index: 2})}, }); @@ -3650,7 +3650,7 @@ describe('metrics reducers', () => { }, }, }, - linkedTimeSelection: {start: {step: 20}, end: null}, + linkedTimeSelection: {start: null, end: {step: 20}}, cardStepIndex: {[imageCardId]: buildStepIndexMetadata({index: 2})}, }); @@ -3668,8 +3668,8 @@ describe('metrics reducers', () => { const state2 = reducers(state1, actions.linkedTimeToggled({})); expect(state2.linkedTimeSelection).toEqual({ - start: {step: 0}, - end: null, + start: null, + end: {step: 0}, }); }); @@ -3681,24 +3681,24 @@ describe('metrics reducers', () => { const state2 = reducers(state1, actions.linkedTimeToggled({})); expect(state2.linkedTimeSelection).toEqual({ - start: {step: 10}, - end: null, + start: null, + end: {step: 100}, }); }); it('dose not update linkedTimeSelection on toggling when it is pre-existed', () => { const state1 = buildMetricsState({ - linkedTimeSelection: {start: {step: 20}, end: null}, + linkedTimeSelection: {start: null, end: {step: 20}}, }); const state2 = reducers(state1, actions.linkedTimeToggled({})); expect(state2.linkedTimeSelection).toEqual({ - start: {step: 20}, - end: null, + start: null, + end: {step: 20}, }); }); - it('enables rangeSelection if linkedTimeSelection has an end step', () => { + it('enables rangeSelection if linkedTimeSelection has a start step', () => { const state1 = buildMetricsState({ stepSelectorEnabled: true, rangeSelectionEnabled: false, @@ -3714,14 +3714,14 @@ describe('metrics reducers', () => { expect(state2.linkedTimeEnabled).toBeTrue(); }); - it('does not enable rangeSelection if linkedTimeSelection does not have an end step', () => { + it('does not enable rangeSelection if linkedTimeSelection does not have a start step', () => { const state1 = buildMetricsState({ stepSelectorEnabled: true, rangeSelectionEnabled: false, linkedTimeEnabled: false, linkedTimeSelection: { - start: {step: 5}, - end: null, + start: null, + end: {step: 5}, }, }); diff --git a/tensorboard/webapp/metrics/store/metrics_selectors.ts b/tensorboard/webapp/metrics/store/metrics_selectors.ts index 35dc62a2bb..c9637bcc2c 100644 --- a/tensorboard/webapp/metrics/store/metrics_selectors.ts +++ b/tensorboard/webapp/metrics/store/metrics_selectors.ts @@ -438,10 +438,10 @@ export const getMetricsLinkedTimeSelectionSetting = createSelector( (state, stepMinMax): TimeSelection => { if (!state.linkedTimeSelection) { return { - start: { - step: stepMinMax.min, + start: null, + end: { + step: stepMinMax.max, }, - end: null, }; } @@ -451,7 +451,7 @@ export const getMetricsLinkedTimeSelectionSetting = createSelector( /** * Returns linked time selection set by user. If linkedTime is disabled, it returns - * `null`. Also, when range selection mode is disabled, it returns `end=null` + * `null`. Also, when range selection mode is disabled, it returns `start=null` * even if it has value set. * * Virtually all views should use this selector. @@ -588,8 +588,9 @@ export const getMetricsCardTimeSelection = createSelector( globalRangeSelectionEnabled ); - const startStep = cardState.timeSelection?.start.step ?? minMaxStep.minStep; - const endStep = cardState.timeSelection?.end?.step ?? minMaxStep.maxStep; + const startStep = + cardState.timeSelection?.start?.step ?? minMaxStep.minStep; + const endStep = cardState.timeSelection?.end.step ?? minMaxStep.maxStep; // The default time selection return formatTimeSelection( diff --git a/tensorboard/webapp/metrics/store/metrics_selectors_test.ts b/tensorboard/webapp/metrics/store/metrics_selectors_test.ts index a6cba2e60c..f5fef66198 100644 --- a/tensorboard/webapp/metrics/store/metrics_selectors_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_selectors_test.ts @@ -549,7 +549,7 @@ describe('metrics selectors', () => { ).toBeUndefined(); }); - it('uses max step as end value if none exists', () => { + it('uses min step as start value if none exists', () => { const state = appStateFromMetricsState( buildMetricsState({ ...partialState, @@ -558,12 +558,12 @@ describe('metrics selectors', () => { card1: { stepSelectionOverride: CardFeatureOverride.OVERRIDE_AS_ENABLED, dataMinMax: { - minStep: 0, + minStep: 5, maxStep: 10, }, timeSelection: { - start: {step: 0}, - end: null, + start: null, + end: {step: 10}, }, }, }, @@ -571,7 +571,7 @@ describe('metrics selectors', () => { ); expect(selectors.getMetricsCardTimeSelection(state, 'card1')).toEqual({ - start: {step: 0}, + start: {step: 5}, end: {step: 10}, }); }); @@ -599,7 +599,7 @@ describe('metrics selectors', () => { }); }); - it('removes end value if range selection is overridden as disabled', () => { + it('removes start value if range selection is overridden as disabled', () => { const state = appStateFromMetricsState( buildMetricsState({ ...partialState, @@ -610,8 +610,8 @@ describe('metrics selectors', () => { rangeSelectionOverride: CardFeatureOverride.OVERRIDE_AS_DISABLED, dataMinMax: { - minStep: 0, - maxStep: 5, + minStep: 5, + maxStep: 10, }, }, }, @@ -619,12 +619,12 @@ describe('metrics selectors', () => { ); expect(selectors.getMetricsCardTimeSelection(state, 'card1')).toEqual({ - start: {step: 0}, - end: null, + start: null, + end: {step: 10}, }); }); - it('removes end value if range selection is globally disabled', () => { + it('removes start value if range selection is globally disabled', () => { const state = appStateFromMetricsState( buildMetricsState({ ...partialState, @@ -633,8 +633,8 @@ describe('metrics selectors', () => { card1: { stepSelectionOverride: CardFeatureOverride.OVERRIDE_AS_ENABLED, dataMinMax: { - minStep: 0, - maxStep: 5, + minStep: 5, + maxStep: 10, }, }, }, @@ -642,12 +642,12 @@ describe('metrics selectors', () => { ); expect(selectors.getMetricsCardTimeSelection(state, 'card1')).toEqual({ - start: {step: 0}, - end: null, + start: null, + end: {step: 10}, }); }); - it('does not remove end value if range selection is overridden as enabled', () => { + it('does not remove start value if range selection is overridden as enabled', () => { const state = appStateFromMetricsState( buildMetricsState({ ...partialState, @@ -733,7 +733,7 @@ describe('metrics selectors', () => { }); }); - it('removes end value if global range selection is disabled', () => { + it('removes start value if global range selection is disabled', () => { const state = appStateFromMetricsState( buildMetricsState({ ...partialState, @@ -750,12 +750,12 @@ describe('metrics selectors', () => { ); expect(selectors.getMetricsCardTimeSelection(state, 'card1')).toEqual({ - start: {step: 0}, - end: null, + start: null, + end: {step: 5}, }); }); - it('maintains end value if local range selection is overridden as disabled', () => { + it('maintains start value if local range selection is overridden as disabled', () => { const state = appStateFromMetricsState( buildMetricsState({ ...partialState, @@ -1381,7 +1381,7 @@ describe('metrics selectors', () => { selectors.getMetricsLinkedTimeSelectionSetting.release(); }); - it('returns value with start step from the dataset when linked time selection is null', () => { + it('returns value with end step from the dataset when linked time selection is null', () => { const state = appStateFromMetricsState( buildMetricsState({ linkedTimeSelection: null, @@ -1392,8 +1392,8 @@ describe('metrics selectors', () => { }) ); expect(selectors.getMetricsLinkedTimeSelectionSetting(state)).toEqual({ - start: {step: 0}, - end: null, + start: null, + end: {step: 1000}, }); }); diff --git a/tensorboard/webapp/metrics/store/metrics_store_internal_utils.ts b/tensorboard/webapp/metrics/store/metrics_store_internal_utils.ts index efd57e56bc..1e2513429f 100644 --- a/tensorboard/webapp/metrics/store/metrics_store_internal_utils.ts +++ b/tensorboard/webapp/metrics/store/metrics_store_internal_utils.ts @@ -431,10 +431,10 @@ export function generateNextCardStepIndexFromLinkedTimeSelection( const steps = getImageCardSteps(cardId, cardMetadataMap, timeSeriesData); let nextStepIndexMetaData: CardStepIndexMetaData | null = null; - if (timeSelection.end === null) { + if (timeSelection.start === null) { // Single Selection nextStepIndexMetaData = getNextImageCardStepIndexFromSingleSelection( - timeSelection.start.step, + timeSelection.end.step, steps ); } else { @@ -492,10 +492,10 @@ function getSelectedSteps( ) { if (!timeSelection) return []; - // Single selection: returns start step if matching any step in the list, otherwise returns nothing. - if (timeSelection.end === null) { - if (steps.indexOf(timeSelection.start.step) !== -1) - return [timeSelection.start.step]; + // Single selection: returns end step if matching any step in the list, otherwise returns nothing. + if (timeSelection.start === null) { + if (steps.indexOf(timeSelection.end.step) !== -1) + return [timeSelection.end.step]; return []; } @@ -511,8 +511,8 @@ function getSelectedSteps( /** * Gets next stepIndex for an image card based on single selection. Returns null if nothing should change. - * @param selectedStep The selected step from linked time selection. It is equivalent to start step here - * since there is no `end` in linked time selection when it is single seleciton. + * @param selectedStep The selected step from linked time selection. It is equivalent to end step here + * since there is no `start` in linked time selection when it is single seleciton. */ function getNextImageCardStepIndexFromSingleSelection( selectedStep: number, @@ -524,7 +524,7 @@ function getNextImageCardStepIndexFromSingleSelection( return {index: maybeMatchedStepIndex, isClosest: false}; } - // Checks if start step is "close" enough to a step value and move it + // Checks if end step is "close" enough to a step value and move it for (let i = 0; i < steps.length - 1; i++) { const currentStep = steps[i]; const nextStep = steps[i + 1]; 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 382e02f02c..d5bb8aaf2f 100644 --- a/tensorboard/webapp/metrics/store/metrics_store_internal_utils_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_store_internal_utils_test.ts @@ -814,7 +814,7 @@ describe('metrics store utils', () => { {[imageCardId]: buildStepIndexMetadata({index: 3})}, cardMetadataMap, timeSeriesData, - {start: {step: 20}, end: null} + {start: null, end: {step: 20}} ); expect(nextCardStepIndex).toEqual({ @@ -851,7 +851,7 @@ describe('metrics store utils', () => { }, images: {}, }; - const linkedTimeSelection = {start: {step: 20}, end: null}; + const linkedTimeSelection = {start: null, end: {step: 20}}; const nextCardStepIndex = generateNextCardStepIndexFromLinkedTimeSelection( @@ -871,7 +871,7 @@ describe('metrics store utils', () => { {[imageCardId]: buildStepIndexMetadata({index: 3})}, cardMetadataMap, timeSeriesData, - {start: {step: 15}, end: null} + {start: null, end: {step: 15}} ); expect(nextCardStepIndex).toEqual({ @@ -1013,7 +1013,7 @@ describe('metrics store utils', () => { describe('getSelectedSteps', () => { it(`gets one selected step on single selection`, () => { - const linkedTimeSelection = {start: {step: 10}, end: null}; + const linkedTimeSelection = {start: null, end: {step: 10}}; const steps = [10]; expect(getSelectedSteps(linkedTimeSelection, steps)).toEqual([10]); @@ -1043,7 +1043,7 @@ describe('metrics store utils', () => { }); it(`gets empty selected steps when single linked time selection does not contain any steps`, () => { - const linkedTimeSelection = {start: {step: 10}, end: null}; + const linkedTimeSelection = {start: null, end: {step: 10}}; const steps = [5, 20, 30]; expect(getSelectedSteps(linkedTimeSelection, steps)).toEqual([]); diff --git a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ts b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ts index 1cf04ffb06..17e655e699 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ts @@ -78,8 +78,8 @@ export class HistogramCardComponent { return null; } return { - start: {step: timeSelection.startStep}, - end: timeSelection.endStep ? {step: timeSelection.endStep} : null, + start: timeSelection.startStep ? {step: timeSelection.startStep} : null, + end: {step: timeSelection.endStep}, }; } } diff --git a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_container.ts b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_container.ts index 5eb35a2e28..eb94028d07 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_container.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_container.ts @@ -54,8 +54,8 @@ import {CardRenderer} from '../metrics_view_types'; import {getTagDisplayName} from '../utils'; import { maybeClipTimeSelectionView, - maybeOmitTimeSelectionEnd, - maybeSetClosestStartStep, + maybeOmitTimeSelectionStart, + maybeSetClosestEndStep, TimeSelectionView, } from './utils'; @@ -186,7 +186,7 @@ export class HistogramCardContainer implements CardRenderer, OnInit { minStep = Math.min(step, minStep); maxStep = Math.max(step, maxStep); } - const formattedTimeSelection = maybeOmitTimeSelectionEnd( + const formattedTimeSelection = maybeOmitTimeSelectionStart( linkedTimeSelection, rangeSelectionEnabled ); @@ -196,7 +196,7 @@ export class HistogramCardContainer implements CardRenderer, OnInit { maxStep ); - return maybeSetClosestStartStep(linkedTimeSelectionView, steps); + return maybeSetClosestEndStep(linkedTimeSelectionView, steps); }) ); @@ -209,8 +209,8 @@ export class HistogramCardContainer implements CardRenderer, OnInit { linkedTimeSelection && linkedTimeSelectionView && !linkedTimeSelectionView.clipped && - linkedTimeSelection.end === null && - linkedTimeSelection.start.step !== linkedTimeSelectionView.startStep + linkedTimeSelection.start === null && + linkedTimeSelection.end.step !== linkedTimeSelectionView.endStep ); }) ); diff --git a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_test.ts b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_test.ts index 22ac970d64..877793c4de 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_test.ts @@ -68,8 +68,8 @@ class TestableHistogramWidget { @Input() name!: string; @Input() data!: HistogramData; @Input() timeSelection!: { - start: {step: number}; - end: {step: number} | null; + start: {step: number} | null; + end: {step: number}; } | null; @Output() onLinkedTimeToggled = new EventEmitter(); @@ -294,8 +294,8 @@ describe('histogram card', () => { it('dispatches timeSelectionChanged when HistogramComponent emits onLinkedTimeSelectionChanged event', () => { provideMockCardSeriesData(selectSpy, PluginType.HISTOGRAMS, 'card1'); store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { - start: {step: 5}, - end: null, + start: null, + end: {step: 5}, }); const fixture = createHistogramCardContainer(); fixture.detectChanges(); @@ -308,15 +308,15 @@ describe('histogram card', () => { By.directive(TestableHistogramWidget) ).componentInstance; histogramWidget.onLinkedTimeSelectionChanged.emit({ - timeSelection: {start: {step: 5}, end: null}, + timeSelection: {start: null, end: {step: 5}}, affordance: TimeSelectionAffordance.FOB, }); expect(dispatchedActions).toEqual([ timeSelectionChanged({ timeSelection: { - start: {step: 5}, - end: null, + start: null, + end: {step: 5}, }, affordance: TimeSelectionAffordance.FOB, }), @@ -338,8 +338,8 @@ describe('histogram card', () => { it('passes closest step linked time parameter to histogram viz', () => { provideMockCardSeriesData(selectSpy, PluginType.HISTOGRAMS, 'card1'); store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { - start: {step: 5}, - end: null, + start: null, + end: {step: 5}, }); const fixture = createHistogramCardContainer(); fixture.detectChanges(); @@ -349,8 +349,8 @@ describe('histogram card', () => { ); expect(viz.componentInstance.timeSelection).toEqual({ // Steps are [0, 1, 99] in mock data - start: {step: 1}, - end: null, + start: null, + end: {step: 1}, }); }); @@ -372,13 +372,13 @@ describe('histogram card', () => { }); }); - it('removes end step when range selection is disabled', () => { + it('removes start step when range selection is disabled', () => { provideMockCardSeriesData( selectSpy, PluginType.HISTOGRAMS, 'card1', undefined, - [buildHistogramStepData({step: 5}), buildHistogramStepData({step: 15})] + [buildHistogramStepData({step: 5}), buildHistogramStepData({step: 10})] ); store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { start: {step: 5}, @@ -392,8 +392,8 @@ describe('histogram card', () => { By.directive(TestableHistogramWidget) ); expect(viz.componentInstance.timeSelection).toEqual({ - start: {step: 5}, - end: null, + start: null, + end: {step: 10}, }); }); @@ -512,8 +512,8 @@ describe('histogram card', () => { it('renders warning when no data on the selected step', () => { store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { - start: {step: 99}, - end: null, + start: null, + end: {step: 99}, }); const fixture = createHistogramCardContainer(); fixture.detectChanges(); @@ -528,8 +528,8 @@ describe('histogram card', () => { it('does not render warning when data exist on selected step', () => { store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { - start: {step: 100}, - end: null, + start: null, + end: {step: 100}, }); const fixture = createHistogramCardContainer(); fixture.detectChanges(); @@ -544,8 +544,8 @@ describe('histogram card', () => { it('does not render warning when time selection is clipped', () => { store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { - start: {step: 49}, - end: null, + start: null, + end: {step: 49}, }); const fixture = createHistogramCardContainer(); fixture.detectChanges(); @@ -577,8 +577,8 @@ describe('histogram card', () => { it('dispatches stepSelectorToggled when HistogramComponent emits the onLinkedTimeToggled event', () => { provideMockCardSeriesData(selectSpy, PluginType.HISTOGRAMS, 'card1'); store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { - start: {step: 5}, - end: null, + start: null, + end: {step: 5}, }); const fixture = createHistogramCardContainer(); fixture.detectChanges(); diff --git a/tensorboard/webapp/metrics/views/card_renderer/image_card_component.ng.html b/tensorboard/webapp/metrics/views/card_renderer/image_card_component.ng.html index 5badae716b..ec9c4e51fa 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/image_card_component.ng.html +++ b/tensorboard/webapp/metrics/views/card_renderer/image_card_component.ng.html @@ -82,7 +82,7 @@
- + { if (!linkedTimeSelection) return []; - if (linkedTimeSelection.endStep === null) { - if (steps.indexOf(linkedTimeSelection.startStep) !== -1) - return [linkedTimeSelection.startStep]; + if (linkedTimeSelection.startStep === null) { + if (steps.indexOf(linkedTimeSelection.endStep) !== -1) + return [linkedTimeSelection.endStep]; return []; } return steps.filter( (step) => - step >= linkedTimeSelection.startStep && - step <= linkedTimeSelection.endStep! + step >= linkedTimeSelection.startStep! && + step <= linkedTimeSelection.endStep ); }) ); diff --git a/tensorboard/webapp/metrics/views/card_renderer/image_card_test.ts b/tensorboard/webapp/metrics/views/card_renderer/image_card_test.ts index 1bdf86ce0d..119d95b711 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/image_card_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/image_card_test.ts @@ -515,8 +515,8 @@ describe('image card', () => { it('renders a single tick on linked time selection', () => { store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { - start: {step: 10}, - end: null, + start: null, + end: {step: 10}, }); const fixture = createImageCardContainer('card1'); @@ -533,8 +533,8 @@ describe('image card', () => { it('renders a single tick at correct propositional position', () => { store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { - start: {step: 20}, - end: null, + start: null, + end: {step: 20}, }); const fixture = createImageCardContainer('card1'); @@ -770,8 +770,8 @@ describe('image card', () => { it('does not render range slider on single selection', () => { store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { - start: {step: 15}, - end: null, + start: null, + end: {step: 15}, }); const fixture = createImageCardContainer('card1'); @@ -788,8 +788,8 @@ describe('image card', () => { describe('single selection', () => { it('does not move slider thumb to larger closest step when it is clipped', () => { store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { - start: {step: 9}, - end: null, + start: null, + end: {step: 9}, }); const timeSeries = [ {wallTime: 100, imageId: 'ImageId1', step: 10}, @@ -814,8 +814,8 @@ describe('image card', () => { it('does not move slider thumb to smaller closest step when it is clipped', () => { store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { // Linked time is clipped since step 41 is larger than the largest step 40. - start: {step: 41}, - end: null, + start: null, + end: {step: 41}, }); const timeSeries = [ {wallTime: 100, imageId: 'ImageId1', step: 10}, @@ -840,8 +840,8 @@ describe('image card', () => { it('does not move slider thumb to larger closest step when it is clipped', () => { store.overrideSelector(selectors.getMetricsLinkedTimeSelection, { // Linked time is clipped since step 9 is smaller than the smallest step 10. - start: {step: 9}, - end: null, + start: null, + end: {step: 9}, }); const timeSeries = [ {wallTime: 100, imageId: 'ImageId1', step: 10}, diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html index 49e27c00c5..41ffb455b0 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html @@ -228,31 +228,31 @@ >
diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts index 9fb4f737ff..86df5b8a43 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts @@ -462,7 +462,7 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy { this.store.select(getRangeSelectionHeaders), ]).pipe( map(([timeSelection, singleSelectionHeaders, rangeSelectionHeaders]) => { - if (!timeSelection || timeSelection.end === null) { + if (!timeSelection || timeSelection.start === null) { return singleSelectionHeaders; } else { return rangeSelectionHeaders; diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts index 9e71487b99..e893df5f1a 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts @@ -110,25 +110,22 @@ export class ScalarCardDataTable { if (!this.stepOrLinkedTimeSelection) { return []; } - const startStep = this.stepOrLinkedTimeSelection.start.step; - const endStep = this.stepOrLinkedTimeSelection.end?.step; + const startStep = this.stepOrLinkedTimeSelection.start?.step; + const endStep = this.stepOrLinkedTimeSelection.end.step; const dataTableData: SelectedStepRunData[] = this.dataSeries .filter((datum) => { return isDatumVisible(datum, this.chartMetadataMap); }) .map((datum) => { const metadata = this.chartMetadataMap[datum.id]; - const closestStartPointIndex = findClosestIndex( - datum.points, - startStep - ); - const closestStartPoint = datum.points[closestStartPointIndex]; - let closestEndPoint: ScalarCardPoint | null = null; - let closestEndPointIndex: number | null = null; - if (endStep !== null && endStep !== undefined) { - closestEndPointIndex = findClosestIndex(datum.points, endStep); - closestEndPoint = datum.points[closestEndPointIndex]; + let closestStartPoint: ScalarCardPoint | null = null; + let closestStartPointIndex: number | null = null; + if (startStep !== null && startStep !== undefined) { + closestStartPointIndex = findClosestIndex(datum.points, startStep); + closestStartPoint = datum.points[closestStartPointIndex]; } + const closestEndPointIndex = findClosestIndex(datum.points, endStep); + const closestEndPoint = datum.points[closestEndPointIndex]; const selectedStepData: SelectedStepRunData = { id: datum.id, }; @@ -143,45 +140,44 @@ export class ScalarCardDataTable { selectedStepData.RUN = `${alias}${metadata.displayName}`; continue; case ColumnHeaderType.STEP: - selectedStepData.STEP = closestStartPoint.step; + selectedStepData.STEP = closestEndPoint.step; continue; case ColumnHeaderType.VALUE: - selectedStepData.VALUE = closestStartPoint.value; + selectedStepData.VALUE = closestEndPoint.value; continue; case ColumnHeaderType.RELATIVE_TIME: - selectedStepData.RELATIVE_TIME = - closestStartPoint.relativeTimeInMs; + selectedStepData.RELATIVE_TIME = closestEndPoint.relativeTimeInMs; continue; case ColumnHeaderType.SMOOTHED: - selectedStepData.SMOOTHED = closestStartPoint.y; + selectedStepData.SMOOTHED = closestEndPoint.y; continue; case ColumnHeaderType.VALUE_CHANGE: - if (!closestEndPoint) { + if (closestStartPoint === null) { continue; } selectedStepData.VALUE_CHANGE = closestEndPoint.y - closestStartPoint.y; continue; case ColumnHeaderType.START_STEP: + if (closestStartPoint === null) { + continue; + } selectedStepData.START_STEP = closestStartPoint.step; continue; case ColumnHeaderType.END_STEP: - if (!closestEndPoint) { - continue; - } selectedStepData.END_STEP = closestEndPoint.step; continue; case ColumnHeaderType.START_VALUE: + if (closestStartPoint === null) { + continue; + } selectedStepData.START_VALUE = closestStartPoint.y; continue; case ColumnHeaderType.END_VALUE: - if (!closestEndPoint) { - continue; - } selectedStepData.END_VALUE = closestEndPoint.y; continue; case ColumnHeaderType.MIN_VALUE: - if (!closestEndPointIndex) { + if (closestStartPointIndex === null) { continue; } selectedStepData.MIN_VALUE = this.getMinPointInRange( @@ -191,7 +187,7 @@ export class ScalarCardDataTable { ).y; continue; case ColumnHeaderType.MAX_VALUE: - if (!closestEndPointIndex) { + if (closestStartPointIndex === null) { continue; } selectedStepData.MAX_VALUE = this.getMaxPointInRange( @@ -201,14 +197,14 @@ export class ScalarCardDataTable { ).y; continue; case ColumnHeaderType.PERCENTAGE_CHANGE: - if (!closestEndPoint) { + if (closestStartPoint === null) { continue; } selectedStepData.PERCENTAGE_CHANGE = (closestEndPoint.y - closestStartPoint.y) / closestStartPoint.y; continue; case ColumnHeaderType.STEP_AT_MAX: - if (!closestEndPointIndex) { + if (closestStartPointIndex === null) { continue; } selectedStepData.STEP_AT_MAX = this.getMaxPointInRange( @@ -218,7 +214,7 @@ export class ScalarCardDataTable { ).step; continue; case ColumnHeaderType.STEP_AT_MIN: - if (!closestEndPointIndex) { + if (closestStartPointIndex === null) { continue; } selectedStepData.STEP_AT_MIN = this.getMinPointInRange( @@ -228,7 +224,7 @@ export class ScalarCardDataTable { ).step; continue; case ColumnHeaderType.MEAN: - if (!closestEndPointIndex) { + if (closestStartPointIndex === null) { continue; } selectedStepData.MEAN = this.getMean( @@ -238,7 +234,7 @@ export class ScalarCardDataTable { ); continue; case ColumnHeaderType.RAW_CHANGE: - if (!closestEndPoint) { + if (closestStartPoint === null) { continue; } selectedStepData.RAW_CHANGE = @@ -284,9 +280,10 @@ export class ScalarCardDataTable { orderColumns(headers: ColumnHeader[]) { this.editColumnHeaders.emit({ headers: headers, - dataTableMode: this.stepOrLinkedTimeSelection.end - ? DataTableMode.RANGE - : DataTableMode.SINGLE, + dataTableMode: + this.stepOrLinkedTimeSelection.start !== null + ? DataTableMode.RANGE + : DataTableMode.SINGLE, }); } } diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_fob_controller.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_fob_controller.ts index 315efcfd7b..e8b6345f56 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_fob_controller.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_fob_controller.ts @@ -74,24 +74,24 @@ export class ScalarCardFobController { prospectiveStep: number | null = null; getAxisPositionFromStartStep() { - if (!this.timeSelection) { - return ''; + if (!this.timeSelection || this.timeSelection.start === null) { + return null; } return this.scale.forward( this.minMaxHorizontalViewExtend, [0, this.axisSize], - this.timeSelection.start.step + this.timeSelection?.start.step ?? this.minMaxStep.maxStep ); } getAxisPositionFromEndStep() { - if (!this.timeSelection?.end) { - return null; + if (!this.timeSelection) { + return ''; } return this.scale.forward( this.minMaxHorizontalViewExtend, [0, this.axisSize], - this.timeSelection?.end.step ?? this.minMaxStep.maxStep + this.timeSelection.end.step ); } diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_fob_test.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_fob_test.ts index 288bc459e2..b4536e9296 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_fob_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_fob_test.ts @@ -39,8 +39,8 @@ describe('ScalarFobController', () => { }): ComponentFixture { const fixture = TestBed.createComponent(ScalarCardFobController); fixture.componentInstance.timeSelection = input.timeSelection ?? { - start: {step: 200}, - end: null, + start: null, + end: {step: 200}, }; fixture.componentInstance.minMaxHorizontalViewExtend = input.minMax ?? [ diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts index 2f66c54ad9..7391ccf1cf 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts @@ -2326,8 +2326,8 @@ describe('scalar card', () => { it('renders fobs', fakeAsync(() => { store.overrideSelector(getMetricsCardTimeSelection, { - start: {step: 20}, - end: null, + start: null, + end: {step: 20}, }); const fixture = createComponent('card1'); fixture.detectChanges(); @@ -2338,8 +2338,8 @@ describe('scalar card', () => { it('does not render fobs when axis type is RELATIVE', fakeAsync(() => { store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 20}, - end: null, + start: null, + end: {step: 20}, }); store.overrideSelector( selectors.getMetricsXAxisType, @@ -2355,8 +2355,8 @@ describe('scalar card', () => { it('does not render fobs when axis type is WALL_TIME', fakeAsync(() => { store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 20}, - end: null, + start: null, + end: {step: 20}, }); store.overrideSelector( selectors.getMetricsXAxisType, @@ -2380,8 +2380,8 @@ describe('scalar card', () => { it('dispatches timeSelectionChanged action when fob is dragged', fakeAsync(() => { store.overrideSelector(getMetricsCardTimeSelection, { - start: {step: 20}, - end: null, + start: null, + end: {step: 20}, }); const fixture = createComponent('card1'); fixture.detectChanges(); @@ -2393,7 +2393,7 @@ describe('scalar card', () => { // Simulate dragging fob to step 25. testController.startDrag( - Fob.START, + Fob.END, TimeSelectionAffordance.FOB, new MouseEvent('mouseDown') ); @@ -2405,8 +2405,8 @@ describe('scalar card', () => { // Simulate ngrx update from mouseMove; store.overrideSelector(getMetricsCardTimeSelection, { - start: {step: 25}, - end: null, + start: null, + end: {step: 25}, }); store.refreshState(); fixture.detectChanges(); @@ -2415,7 +2415,7 @@ describe('scalar card', () => { fixture.detectChanges(); testController.startDrag( - Fob.START, + Fob.END, TimeSelectionAffordance.EXTENDED_LINE, new MouseEvent('mouseDown') ); @@ -2427,8 +2427,8 @@ describe('scalar card', () => { // Simulate ngrx update from mouseMove; store.overrideSelector(getMetricsCardTimeSelection, { - start: {step: 30}, - end: null, + start: null, + end: {step: 30}, }); store.refreshState(); fixture.detectChanges(); @@ -2440,16 +2440,16 @@ describe('scalar card', () => { // Call from first mouseMove. timeSelectionChanged({ timeSelection: { - start: {step: 25}, - end: null, + start: null, + end: {step: 25}, }, cardId: 'card1', }), // Call from first stopDrag. timeSelectionChanged({ timeSelection: { - start: {step: 25}, - end: null, + start: null, + end: {step: 25}, }, affordance: TimeSelectionAffordance.FOB, cardId: 'card1', @@ -2457,16 +2457,16 @@ describe('scalar card', () => { // Call from second mouseMove. timeSelectionChanged({ timeSelection: { - start: {step: 30}, - end: null, + start: null, + end: {step: 30}, }, cardId: 'card1', }), // Call from second stopDrag. timeSelectionChanged({ timeSelection: { - start: {step: 30}, - end: null, + start: null, + end: {step: 30}, }, affordance: TimeSelectionAffordance.EXTENDED_LINE, cardId: 'card1', @@ -2476,8 +2476,8 @@ describe('scalar card', () => { it('toggles step selection when single fob is deselected even when linked time is enabled', fakeAsync(() => { store.overrideSelector(getMetricsCardTimeSelection, { - start: {step: 20}, - end: null, + start: null, + end: {step: 20}, }); const fixture = createComponent('card1'); fixture.detectChanges(); @@ -2534,8 +2534,8 @@ describe('scalar card', () => { it('renders table', fakeAsync(() => { store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 20}, - end: null, + start: null, + end: {step: 20}, }); const fixture = createComponent('card1'); fixture.detectChanges(); @@ -2561,8 +2561,8 @@ describe('scalar card', () => { it('does not render table when axis type is RELATIVE', fakeAsync(() => { store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 20}, - end: null, + start: null, + end: {step: 20}, }); store.overrideSelector( selectors.getMetricsXAxisType, @@ -2580,8 +2580,8 @@ describe('scalar card', () => { it('does not render table when axis type is WALL_TIME', fakeAsync(() => { store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 20}, - end: null, + start: null, + end: {step: 20}, }); store.overrideSelector( selectors.getMetricsXAxisType, @@ -2720,8 +2720,8 @@ describe('scalar card', () => { ); store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 2}, - end: null, + start: null, + end: {step: 2}, }); const fixture = createComponent('card1'); @@ -2983,8 +2983,8 @@ describe('scalar card', () => { ); store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 18}, - end: null, + start: null, + end: {step: 18}, }); const fixture = createComponent('card1'); @@ -3049,7 +3049,7 @@ describe('scalar card', () => { expect(data[1].END_STEP).toEqual(25); })); - it('selects largest points when time selection startStep is greater than any points step', fakeAsync(() => { + it('selects largest points when time selection endStep is greater than any points step', fakeAsync(() => { const runToSeries = { run1: [ {wallTime: 1, value: 1, step: 1}, @@ -3078,8 +3078,8 @@ describe('scalar card', () => { ); store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 100}, - end: null, + start: null, + end: {step: 100}, }); const fixture = createComponent('card1'); @@ -3095,7 +3095,7 @@ describe('scalar card', () => { expect(data[1].STEP).toEqual(50); })); - it('selects smallest points when time selection startStep is less than any points step', fakeAsync(() => { + it('selects smallest points when time selection endStep is less than any points step', fakeAsync(() => { const runToSeries = { run1: [ {wallTime: 1, value: 1, step: 10}, @@ -3123,8 +3123,8 @@ describe('scalar card', () => { ]) ); store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 1}, - end: null, + start: null, + end: {step: 1}, }); const fixture = createComponent('card1'); @@ -3172,8 +3172,8 @@ describe('scalar card', () => { eid2: {aliasText: 'test alias 2', aliasNumber: 200}, }); store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 1}, - end: null, + start: null, + end: {step: 1}, }); selectSpy .withArgs(selectors.getExperimentIdForRunId, {runId: 'run1'}) @@ -3222,8 +3222,8 @@ describe('scalar card', () => { new Map([['run1', true]]) ); store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 20}, - end: null, + start: null, + end: {step: 20}, }); const fixture = createComponent('card1'); @@ -3266,8 +3266,8 @@ describe('scalar card', () => { ); store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 1}, - end: null, + start: null, + end: {step: 1}, }); const fixture = createComponent('card1'); @@ -3311,8 +3311,8 @@ describe('scalar card', () => { ); store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 1}, - end: null, + start: null, + end: {step: 1}, }); const fixture = createComponent('card1'); @@ -3364,8 +3364,8 @@ describe('scalar card', () => { ); store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 1}, - end: null, + start: null, + end: {step: 1}, }); const fixture = createComponent('card1'); @@ -3421,8 +3421,8 @@ describe('scalar card', () => { ); store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 1}, - end: null, + start: null, + end: {step: 1}, }); const fixture = createComponent('card1'); @@ -3528,13 +3528,13 @@ describe('scalar card', () => { ); store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 2}, - end: null, + start: null, + end: {step: 2}, }); store.overrideSelector(getMetricsCardTimeSelection, { - start: {step: 2}, - end: null, + start: null, + end: {step: 2}, }); store.overrideSelector(getCardStateMap, { card1: {}, @@ -3690,7 +3690,7 @@ describe('scalar card', () => { const testController = fixture.debugElement.query( By.directive(CardFobControllerComponent) ).componentInstance; - testController.onProspectiveStepChanged.emit(10); + testController.onProspectiveStepChanged.emit(25); fixture.detectChanges(); // One prospective fob @@ -3699,28 +3699,28 @@ describe('scalar card', () => { ); expect(fobs.length).toEqual(1); - // Click the prospective fob to set the start time + // Click the prospective fob to set the end time testController.prospectiveFobClicked(new MouseEvent('mouseclick')); store.overrideSelector(getMetricsCardTimeSelection, { - start: {step: 10}, - end: null, + start: null, + end: {step: 25}, }); store.refreshState(); fixture.detectChanges(); - // One start fob + // One end fob fobs = fixture.debugElement.queryAll(By.directive(CardFobComponent)); expect(fobs.length).toEqual(1); fixture.detectChanges(); - // One start fob + 1 prospective fob - testController.onProspectiveStepChanged.emit(25); + // One end fob + 1 prospective fob + testController.onProspectiveStepChanged.emit(10); fixture.detectChanges(); fobs = fixture.debugElement.queryAll(By.directive(CardFobComponent)); expect(fobs.length).toEqual(2); - // Click the prospective fob to set the end time + // Click the prospective fob to set the start time testController.prospectiveFobClicked(new MouseEvent('mouseclick')); store.overrideSelector(getMetricsCardTimeSelection, { start: {step: 10}, @@ -3737,8 +3737,8 @@ describe('scalar card', () => { expect(dispatchedActions).toEqual([ timeSelectionChanged({ timeSelection: { - start: {step: 10}, - end: null, + start: null, + end: {step: 25}, }, affordance: TimeSelectionAffordance.FOB_ADDED, cardId: 'card1', @@ -3774,8 +3774,8 @@ describe('scalar card', () => { it('dispatches timeSelectionChanged actions when fob is dragged while linked time is enabled', fakeAsync(() => { store.overrideSelector(getMetricsLinkedTimeEnabled, true); store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 20}, - end: null, + start: null, + end: {step: 20}, }); const fixture = createComponent('card1'); fixture.detectChanges(); @@ -3787,7 +3787,7 @@ describe('scalar card', () => { // Simulate dragging fob to step 25. testController.startDrag( - Fob.START, + Fob.END, TimeSelectionAffordance.FOB, new MouseEvent('mouseDown') ); @@ -3799,8 +3799,8 @@ describe('scalar card', () => { // Simulate ngrx update from mouseMove; store.overrideSelector(getMetricsLinkedTimeSelection, { - start: {step: 25}, - end: null, + start: null, + end: {step: 25}, }); store.refreshState(); fixture.detectChanges(); @@ -3817,8 +3817,8 @@ describe('scalar card', () => { expect(dispatchedActions).toContain( timeSelectionChanged({ timeSelection: { - start: {step: 25}, - end: null, + start: null, + end: {step: 25}, }, affordance: TimeSelectionAffordance.FOB, cardId: 'card1', @@ -3830,8 +3830,8 @@ describe('scalar card', () => { expect( scalarCardComponent.componentInstance.stepOrLinkedTimeSelection ).toEqual({ - start: {step: 25}, - end: null, + start: null, + end: {step: 25}, }); })); @@ -3847,20 +3847,20 @@ describe('scalar card', () => { // Simulate dragging fob to step 25. testController.startDrag( - Fob.START, + Fob.END, TimeSelectionAffordance.FOB, new MouseEvent('mouseDown') ); const fakeEvent = new MouseEvent('mousemove', { clientX: 25 + controllerStartPosition, - movementX: 1, + movementX: -1, }); testController.mouseMove(fakeEvent); // Simulate ngrx update from mouseMove store.overrideSelector(getMetricsCardTimeSelection, { - start: {step: 25}, - end: null, + start: null, + end: {step: 25}, }); store.refreshState(); fixture.detectChanges(); @@ -3871,15 +3871,15 @@ describe('scalar card', () => { expect(dispatchedActions).toEqual([ timeSelectionChanged({ timeSelection: { - start: {step: 25}, - end: null, + start: null, + end: {step: 25}, }, cardId: 'card1', }), timeSelectionChanged({ timeSelection: { - start: {step: 25}, - end: null, + start: null, + end: {step: 25}, }, affordance: TimeSelectionAffordance.FOB, cardId: 'card1', @@ -3970,8 +3970,8 @@ describe('scalar card', () => { }, }); store.overrideSelector(getMetricsCardTimeSelection, { - start: {step: 1}, - end: null, + start: null, + end: {step: 1}, }); store.overrideSelector(selectors.getMetricsStepSelectorEnabled, true); const fixture = createComponent('card1'); diff --git a/tensorboard/webapp/metrics/views/card_renderer/utils.ts b/tensorboard/webapp/metrics/views/card_renderer/utils.ts index dd645daf1e..ced373ee01 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/utils.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/utils.ts @@ -92,8 +92,8 @@ export function partitionSeries(series: PartialSeries[]): PartitionedSeries[] { } export interface TimeSelectionView { - startStep: number; - endStep: number | null; + startStep: number | null; + endStep: number; clipped: boolean; } @@ -122,14 +122,19 @@ export function maybeClipTimeSelection( timeSelection: TimeSelection, {minStep, maxStep}: MinMaxStep ) { - const start = { - step: clipStepWithinMinMax(timeSelection.start.step, minStep, maxStep), + const start = + timeSelection.start !== null + ? { + step: clipStepWithinMinMax( + timeSelection.start.step, + minStep, + maxStep + ), + } + : null; + const end = { + step: clipStepWithinMinMax(timeSelection.end.step, minStep, maxStep), }; - const end = timeSelection.end - ? { - step: clipStepWithinMinMax(timeSelection.end.step, minStep, maxStep), - } - : null; return { start, end, @@ -141,17 +146,18 @@ export function maybeClipTimeSelectionView( minStep: number, maxStep: number ): TimeSelectionView { - const maybeClippedStartStep = clipStepWithinMinMax( - timeSelection.start.step, + const maybeClippedStartStep = + timeSelection.start !== null + ? clipStepWithinMinMax(timeSelection.start.step, minStep, maxStep) + : null; + const maybeClippedEndStep = clipStepWithinMinMax( + timeSelection.end.step, minStep, maxStep ); - const maybeClippedEndStep = timeSelection.end - ? clipStepWithinMinMax(timeSelection.end.step, minStep, maxStep) - : null; const clipped = - maybeClippedStartStep !== timeSelection.start.step || - maybeClippedEndStep !== (timeSelection.end?.step ?? null); + maybeClippedStartStep !== (timeSelection.start?.step ?? null) || + maybeClippedEndStep !== timeSelection.end.step; return { startStep: maybeClippedStartStep, endStep: maybeClippedEndStep, @@ -160,23 +166,23 @@ export function maybeClipTimeSelectionView( } /** - * Sets startStep of TimeSelectionView to the closest step if the closeset step is not null. + * Sets endStep of TimeSelectionView to the closest step if the closeset step is not null. */ -export function maybeSetClosestStartStep( +export function maybeSetClosestEndStep( timeSelectionView: TimeSelectionView, steps: number[] ): TimeSelectionView { - // Only sets start step on single selection. - if (timeSelectionView.endStep !== null) { + // Only sets end step on single selection. + if (timeSelectionView.startStep !== null) { return timeSelectionView; } - const closestStep = getClosestStep(timeSelectionView.startStep, steps); + const closestStep = getClosestStep(timeSelectionView.endStep, steps); if (closestStep !== null) { - // If the closest step is startStep itself, this is equivalent to timeSelectionView. + // If the closest step is endStep itself, this is equivalent to timeSelectionView. return { ...timeSelectionView, - startStep: closestStep, + endStep: closestStep, }; } @@ -217,11 +223,11 @@ export function isDatumVisible( } /** - * Removes the end step of a time selection if range selection is not enabled + * Removes the start step of a time selection if range selection is not enabled * @param timeSelection * @param rangeSelectionEnabled */ -export function maybeOmitTimeSelectionEnd( +export function maybeOmitTimeSelectionStart( timeSelection: TimeSelection, rangeSelectionEnabled: boolean ): TimeSelection { @@ -230,20 +236,20 @@ export function maybeOmitTimeSelectionEnd( } return { - start: timeSelection.start, - end: null, + start: null, + end: timeSelection.end, }; } /** - * Clips a time selection and potentially removes the end step if range selection is not enabled + * Clips a time selection and potentially removes the start step if range selection is not enabled */ export function formatTimeSelection( timeSelection: TimeSelection, minMaxStep: MinMaxStep, rangeSelectionEnabled: boolean ) { - return maybeOmitTimeSelectionEnd( + return maybeOmitTimeSelectionStart( maybeClipTimeSelection(timeSelection, minMaxStep), rangeSelectionEnabled ); diff --git a/tensorboard/webapp/metrics/views/card_renderer/utils_test.ts b/tensorboard/webapp/metrics/views/card_renderer/utils_test.ts index 8751c201a5..6367a42bf7 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/utils_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/utils_test.ts @@ -20,8 +20,8 @@ import { getClosestStep, getDisplayNameForRun, maybeClipTimeSelectionView, - maybeOmitTimeSelectionEnd, - maybeSetClosestStartStep, + maybeOmitTimeSelectionStart, + maybeSetClosestEndStep, partitionSeries, } from './utils'; @@ -253,51 +253,47 @@ describe('metrics card_renderer utils test', () => { }); }); - describe('#maybeSetClosestStartStep', () => { - it('sets startStep to closest step', () => { + describe('#maybeSetClosestEndStep', () => { + it('sets endStep to closest step', () => { const timeSelectionView = { - startStep: 0, - endStep: null, + startStep: null, + endStep: 0, clipped: false, }; - expect(maybeSetClosestStartStep(timeSelectionView, [10, 20, 30])).toEqual( - { - startStep: 10, - endStep: null, - clipped: false, - } - ); + expect(maybeSetClosestEndStep(timeSelectionView, [10, 20, 30])).toEqual({ + startStep: null, + endStep: 10, + clipped: false, + }); }); - it('does not set startStep on an empty array of steps', () => { + it('does not set endStep on an empty array of steps', () => { const timeSelectionView = { - startStep: 0, - endStep: null, + startStep: null, + endStep: 0, clipped: false, }; - expect(maybeSetClosestStartStep(timeSelectionView, [])).toEqual({ - startStep: 0, - endStep: null, + expect(maybeSetClosestEndStep(timeSelectionView, [])).toEqual({ + startStep: null, + endStep: 0, clipped: false, }); }); - it('does not set startStep when time selection is range selection', () => { + it('does not set endStep when time selection is range selection', () => { const timeSelectionView = { startStep: 0, endStep: 3, clipped: false, }; - expect(maybeSetClosestStartStep(timeSelectionView, [10, 20, 30])).toEqual( - { - startStep: 0, - endStep: 3, - clipped: false, - } - ); + expect(maybeSetClosestEndStep(timeSelectionView, [10, 20, 30])).toEqual({ + startStep: 0, + endStep: 3, + clipped: false, + }); }); }); @@ -335,36 +331,36 @@ describe('metrics card_renderer utils test', () => { }); describe('#maybeClipLinkedTimeSelection', () => { - it('clips to the minStep when time selection start step is smaller than the view extend', () => { + it('clips to the minStep when time selection end step is smaller than the view extend', () => { expect( maybeClipTimeSelectionView( { - start: {step: 0}, - end: null, + start: null, + end: {step: 0}, }, 1, 4 ) ).toEqual({ - startStep: 1, - endStep: null, + startStep: null, + endStep: 1, clipped: true, }); }); - it('clips to maxStep when time selection end step is greater than view extend', () => { + it('clips to maxStep when time selection start step is less than view extend', () => { expect( maybeClipTimeSelectionView( { start: {step: 0}, end: {step: 4}, }, - 0, - 3 + 1, + 4 ) ).toEqual({ - startStep: 0, - endStep: 3, + startStep: 1, + endStep: 4, clipped: true, }); }); @@ -373,15 +369,15 @@ describe('metrics card_renderer utils test', () => { expect( maybeClipTimeSelectionView( { - start: {step: 10}, - end: null, + start: null, + end: {step: 10}, }, 0, 20 ) ).toEqual({ - startStep: 10, - endStep: null, + startStep: null, + endStep: 10, clipped: false, }); }); @@ -470,10 +466,10 @@ describe('metrics card_renderer utils test', () => { }); }); - describe('#maybeOmitTimeSelectionEnd', () => { + describe('#maybeOmitTimeSelectionStart', () => { it('does nothing when range selection is enabled', () => { expect( - maybeOmitTimeSelectionEnd( + maybeOmitTimeSelectionStart( { start: {step: 5}, end: {step: 10}, @@ -486,9 +482,9 @@ describe('metrics card_renderer utils test', () => { }); }); - it('sets end step to null when range selection is disabled', () => { + it('sets starts step to null when range selection is disabled', () => { expect( - maybeOmitTimeSelectionEnd( + maybeOmitTimeSelectionStart( { start: {step: 5}, end: {step: 10}, @@ -496,8 +492,8 @@ describe('metrics card_renderer utils test', () => { false ) ).toEqual({ - start: {step: 5}, - end: null, + start: null, + end: {step: 10}, }); }); }); @@ -541,12 +537,12 @@ describe('metrics card_renderer utils test', () => { }); }); - it('does not add an end step when none is provided', () => { + it('does not add an start step when none is provided', () => { expect( formatTimeSelection( { - start: {step: 0}, - end: null, + start: null, + end: {step: 0}, }, { minStep: 100, @@ -555,15 +551,15 @@ describe('metrics card_renderer utils test', () => { true ) ).toEqual({ - start: {step: 100}, - end: null, + start: null, + end: {step: 100}, }); expect( formatTimeSelection( { - start: {step: 100}, - end: null, + start: null, + end: {step: 100}, }, { minStep: 0, @@ -572,15 +568,15 @@ describe('metrics card_renderer utils test', () => { true ) ).toEqual({ - start: {step: 50}, - end: null, + start: null, + end: {step: 50}, }); expect( formatTimeSelection( { - start: {step: 100}, - end: null, + start: null, + end: {step: 100}, }, { minStep: 50, @@ -589,8 +585,8 @@ describe('metrics card_renderer utils test', () => { true ) ).toEqual({ - start: {step: 100}, - end: null, + start: null, + end: {step: 100}, }); }); @@ -651,7 +647,7 @@ describe('metrics card_renderer utils test', () => { }); }); - it('sets end to null when rangeSelection is disabled', () => { + it('sets start to null when rangeSelection is disabled', () => { expect( formatTimeSelection( { @@ -665,8 +661,8 @@ describe('metrics card_renderer utils test', () => { false ) ).toEqual({ - start: {step: 50}, - end: null, + start: null, + end: {step: 100}, }); }); diff --git a/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ng.html b/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ng.html index 6f70463d4e..543f377910 100644 --- a/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ng.html +++ b/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ng.html @@ -55,7 +55,7 @@

General

[checked]="isLinkedTimeEnabled" (change)="linkedTimeToggled.emit()" [disabled]="!isAxisTypeStep()" - >Link by step {{getLinkedTimeSelectionStartStep()}} + >Link by step {{getLinkedTimeSelectionEndStep()}}