Skip to content

Commit

Permalink
Step Selector: Add smoothed column in data table when enabled (#5839)
Browse files Browse the repository at this point in the history
Motivation for features / changes
This change adds the "Smoothed" column to the data table which we expect users to want. It also helps align the data table more closely with the tooltip.

Technical description of changes
Since this column is more dynamic and should only appear when smoothing is enabled I moved the dataHeaders construction to the container where I made it dependent on the smoothingEnabled$ Observable.
  • Loading branch information
JamesHollyer authored and yatbear committed Aug 5, 2022
1 parent 682c93f commit d4338ea
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export class ScalarCardComponent<Downloader> {
@Input() linkedTimeSelection!: TimeSelectionView | null;
@Input() stepSelectorTimeSelection!: TimeSelection;
@Input() minMaxStep!: MinMaxStep;
@Input() dataHeaders!: ColumnHeaders[];

@Output() onFullSizeToggle = new EventEmitter<void>();
@Output() onPinClicked = new EventEmitter<boolean>();
Expand All @@ -116,12 +117,6 @@ export class ScalarCardComponent<Downloader> {

yScaleType = ScaleType.LINEAR;
isViewBoxOverridden: boolean = false;
dataHeaders: ColumnHeaders[] = [
ColumnHeaders.RUN,
ColumnHeaders.VALUE,
ColumnHeaders.STEP,
ColumnHeaders.RELATIVE_TIME,
];

toggleYScaleType() {
this.yScaleType =
Expand Down Expand Up @@ -269,6 +264,9 @@ export class ScalarCardComponent<Downloader> {
selectedStepData.RELATIVE_TIME =
closestStartPoint.relativeTimeInMs;
continue;
case ColumnHeaders.SMOOTHED:
selectedStepData.SMOOTHED = closestStartPoint.y;
continue;
default:
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ import {CardRenderer} from '../metrics_view_types';
import {getTagDisplayName} from '../utils';
import {DataDownloadDialogContainer} from './data_download_dialog_container';
import {
ColumnHeaders,
MinMaxStep,
PartialSeries,
PartitionedSeries,
Expand Down Expand Up @@ -145,6 +146,7 @@ function areSeriesEqual(
[stepSelectorTimeSelection]="stepSelectorTimeSelection$ | async"
[forceSvg]="forceSvg$ | async"
[minMaxStep]="minMaxSteps$ | async"
[dataHeaders]="columnHeaders$ | async"
(onFullSizeToggle)="onFullSizeToggle()"
(onPinClicked)="pinStateChanged.emit($event)"
observeIntersection
Expand Down Expand Up @@ -191,6 +193,7 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy {
linkedTimeSelection$?: Observable<TimeSelectionView | null>;
stepSelectorTimeSelection$?: Observable<TimeSelection | null>;
minMaxSteps$?: Observable<MinMaxStep>;
columnHeaders$?: Observable<ColumnHeaders[]>;

onVisibilityChange({visible}: {visible: boolean}) {
this.isVisible = visible;
Expand Down Expand Up @@ -353,6 +356,21 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy {
})
);

this.columnHeaders$ = this.smoothingEnabled$.pipe(
map((smoothingEnabled) => {
const headers = [
ColumnHeaders.RUN,
ColumnHeaders.VALUE,
ColumnHeaders.STEP,
ColumnHeaders.RELATIVE_TIME,
];
if (smoothingEnabled) {
headers.splice(1, 0, ColumnHeaders.SMOOTHED);
}
return headers;
})
);

this.dataSeries$ = partitionedSeries$.pipe(
// Smooth
combineLatestWith(this.store.select(getMetricsScalarSmoothing)),
Expand Down
57 changes: 57 additions & 0 deletions tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ import {ScalarCardComponent} from './scalar_card_component';
import {ScalarCardContainer} from './scalar_card_container';
import {ScalarCardFobController} from './scalar_card_fob_controller';
import {
ColumnHeaders,
ScalarCardPoint,
ScalarCardSeriesMetadata,
SeriesType,
Expand Down Expand Up @@ -2651,6 +2652,62 @@ describe('scalar card', () => {
expect(data[0].RUN).toEqual('100 test alias 1/Run1 name');
expect(data[1].RUN).toEqual('200 test alias 2/Run2 name');
}));

it('adds smoothed column header when smoothed is enabled', fakeAsync(() => {
store.overrideSelector(selectors.getMetricsScalarSmoothing, 0.8);

const runToSeries = {
run1: [
{wallTime: 1, value: 1, step: 10},
{wallTime: 2, value: 10, step: 20},
{wallTime: 3, value: 20, step: 35},
],
};
provideMockCardRunToSeriesData(
selectSpy,
PluginType.SCALARS,
'card1',
null /* metadataOverride */,
runToSeries
);
store.overrideSelector(
selectors.getCurrentRouteRunSelection,
new Map([['run1', true]])
);
store.overrideSelector(getMetricsLinkedTimeSelection, {
start: {step: 20},
end: null,
});

const fixture = createComponent('card1');
fixture.detectChanges();
const scalarCardComponent = fixture.debugElement.query(
By.directive(ScalarCardComponent)
);

expect(scalarCardComponent.componentInstance.dataHeaders).toContain(
ColumnHeaders.SMOOTHED
);

expect(
scalarCardComponent.componentInstance.getTimeSelectionTableData()[0]
.SMOOTHED
).toBe(6.000000000000001);
}));

it('does not add smoothed column header when smoothed is disabled', fakeAsync(() => {
store.overrideSelector(selectors.getMetricsScalarSmoothing, 0);

const fixture = createComponent('card1');
fixture.detectChanges();
const scalarCardComponent = fixture.debugElement.query(
By.directive(ScalarCardComponent)
);

expect(scalarCardComponent.componentInstance.dataHeaders).not.toContain(
ColumnHeaders.SMOOTHED
);
}));
});

describe('step selector feature integration', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export enum ColumnHeaders {
STEP = 'STEP',
TIME = 'TIME',
VALUE = 'VALUE',
SMOOTHED = 'SMOOTHED',
}

/**
Expand Down
9 changes: 9 additions & 0 deletions tensorboard/webapp/widgets/data_table/data_table_component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ export class DataTableComponent {
return 'Time';
case ColumnHeaders.RELATIVE_TIME:
return 'Relative';
case ColumnHeaders.SMOOTHED:
return 'Smoothed';
default:
return '';
}
Expand Down Expand Up @@ -88,6 +90,13 @@ export class DataTableComponent {
return relativeTimeFormatter.formatReadable(
selectedStepRunData.RELATIVE_TIME as number
);
case ColumnHeaders.SMOOTHED:
if (selectedStepRunData.SMOOTHED === undefined) {
return '';
}
return numberFormatter.formatShort(
selectedStepRunData.SMOOTHED as number
);
default:
return '';
}
Expand Down

0 comments on commit d4338ea

Please sign in to comment.