From 6addbc7686340036b7485ed2264f439e163146d6 Mon Sep 17 00:00:00 2001 From: Stephan Lee Date: Thu, 20 May 2021 23:20:20 -0700 Subject: [PATCH] timeseries: use SI unit for STEP axis Currently, TimeSeries or the new line chart uses scientific notation for display steps. Steps being a very integral number, SI number often is a better fit. This is what traditional TensorBoard used to do, too. --- .../scalar_card_component.ng.html | 2 +- .../card_renderer/scalar_card_component.ts | 13 +++ .../views/card_renderer/scalar_card_test.ts | 88 ++++++++++++++++++- 3 files changed, 100 insertions(+), 3 deletions(-) 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 e81c55db39f..230cf8c1790 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 @@ -104,7 +104,7 @@ [seriesMetadataMap]="chartMetadataMap" [xScaleType]="xScaleType" [yScaleType]="yScaleType" - [customXFormatter]="xAxisType === XAxisType.RELATIVE ? relativeXFormatter : undefined" + [customXFormatter]="getCustomXFormatter()" [ignoreYOutliers]="ignoreOutliers" [tooltipTemplate]="tooltip" (onViewBoxOverridden)="isViewBoxOverridden = $event" diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts index 1c8c94c5920..d46b0355896 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts @@ -26,6 +26,7 @@ import {MatDialog} from '@angular/material/dialog'; import {DataLoadState} from '../../../types/data'; import { + Formatter, numberFormatter, relativeTimeFormatter, siNumberFormatter, @@ -109,6 +110,18 @@ export class ScalarCardComponent { readonly valueFormatter = numberFormatter; readonly stepFormatter = siNumberFormatter; + getCustomXFormatter(): Formatter | undefined { + switch (this.xAxisType) { + case XAxisType.RELATIVE: + return relativeTimeFormatter; + case XAxisType.STEP: + return siNumberFormatter; + case XAxisType.WALL_TIME: + default: + return undefined; + } + } + getCursorAwareTooltipData( tooltipData: TooltipDatum[], cursorLoc: {x: number; y: number} 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 94b34e1c7a8..0925e475e53 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts @@ -44,7 +44,11 @@ import {buildRun} from '../../../runs/store/testing'; import * as selectors from '../../../selectors'; import {MatIconTestingModule} from '../../../testing/mat_icon_module'; import {DataLoadState} from '../../../types/data'; -import {Formatter} from '../../../widgets/line_chart_v2/lib/formatter'; +import { + Formatter, + relativeTimeFormatter, + siNumberFormatter, +} from '../../../widgets/line_chart_v2/lib/formatter'; import { DataSeries, DataSeriesMetadataMap, @@ -55,7 +59,7 @@ import { import {ResizeDetectorTestingModule} from '../../../widgets/resize_detector_testing_module'; import {TruncatedPathModule} from '../../../widgets/text/truncated_path_module'; import {PluginType} from '../../data_source'; -import {getMetricsScalarSmoothing} from '../../store'; +import {getMetricsScalarSmoothing, getMetricsXAxisType} from '../../store'; import { appStateFromMetricsState, buildMetricsState, @@ -395,6 +399,86 @@ describe('scalar card', () => { expect(displayName).toBe('Run1 name'); expect(visible).toBe(true); })); + + describe('custom x axis formatter', () => { + it('uses SI unit formatter when xAxisType is STEP', fakeAsync(() => { + store.overrideSelector(selectors.getMetricsXAxisType, XAxisType.STEP); + + const cardMetadata = { + plugin: PluginType.SCALARS, + tag: 'tagA', + run: null, + }; + provideMockCardRunToSeriesData( + selectSpy, + PluginType.SCALARS, + 'card1', + cardMetadata, + null /* runToSeries */ + ); + + const fixture = createComponent('card1'); + + expect( + fixture.debugElement.query(Selector.LINE_CHART).componentInstance + .customXFormatter + ).toBe(siNumberFormatter); + })); + + it('uses relative time formatter when xAxisType is RELATIVE', fakeAsync(() => { + store.overrideSelector( + selectors.getMetricsXAxisType, + XAxisType.RELATIVE + ); + + const cardMetadata = { + plugin: PluginType.SCALARS, + tag: 'tagA', + run: null, + }; + provideMockCardRunToSeriesData( + selectSpy, + PluginType.SCALARS, + 'card1', + cardMetadata, + null /* runToSeries */ + ); + + const fixture = createComponent('card1'); + + expect( + fixture.debugElement.query(Selector.LINE_CHART).componentInstance + .customXFormatter + ).toBe(relativeTimeFormatter); + })); + + it('does not specify a custom X formatter for xAxisType WALL_TIME', fakeAsync(() => { + store.overrideSelector( + selectors.getMetricsXAxisType, + XAxisType.WALL_TIME + ); + + const cardMetadata = { + plugin: PluginType.SCALARS, + tag: 'tagA', + run: null, + }; + provideMockCardRunToSeriesData( + selectSpy, + PluginType.SCALARS, + 'card1', + cardMetadata, + null /* runToSeries */ + ); + + const fixture = createComponent('card1'); + + expect( + fixture.debugElement.query(Selector.LINE_CHART).componentInstance + .customXFormatter + ).toBe(undefined); + })); + }); }); describe('displayName', () => {