Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

timeseries: add option for monotonic x #4696

Merged
merged 2 commits into from
Feb 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions tensorboard/webapp/metrics/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ export const metricsChangeScalarSmoothing = createAction(
props<{smoothing: number}>()
);

export const metricsScalarPartitionNonMonotonicXToggled = createAction(
'[Metrics] Metrics Setting Partition Non Monotonic X Toggled'
);

export const metricsChangeImageBrightness = createAction(
'[Metrics] Metrics Setting Change Image Brightness',
props<{brightnessInMilli: number}>()
Expand Down
10 changes: 10 additions & 0 deletions tensorboard/webapp/metrics/store/metrics_reducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,16 @@ const reducer = createReducer(
},
};
}),
on(actions.metricsScalarPartitionNonMonotonicXToggled, (state) => {
return {
...state,
settings: {
...state.settings,
scalarPartitionNonMonotonicX: !state.settings
.scalarPartitionNonMonotonicX,
},
};
}),
on(actions.metricsChangeImageBrightness, (state, {brightnessInMilli}) => {
return {
...state,
Expand Down
19 changes: 19 additions & 0 deletions tensorboard/webapp/metrics/store/metrics_reducers_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,25 @@ describe('metrics reducers', () => {
expect(nextState.settings.scalarSmoothing).toBe(0.1);
});

it('toggles Partition X on metricsScalarPartitionNonMonotonicXToggled', () => {
const state1 = buildMetricsState({
settings: buildMetricsSettingsState({
scalarPartitionNonMonotonicX: true,
}),
});
const state2 = reducers(
state1,
actions.metricsScalarPartitionNonMonotonicXToggled()
);
expect(state2.settings.scalarPartitionNonMonotonicX).toBe(false);

const state3 = reducers(
state2,
actions.metricsScalarPartitionNonMonotonicXToggled()
);
expect(state3.settings.scalarPartitionNonMonotonicX).toBe(true);
});

it('changes imageBrightnessInMilli on metricsChangeImageBrightness', () => {
const prevState = buildMetricsState({
settings: buildMetricsSettingsState({
Expand Down
42 changes: 26 additions & 16 deletions tensorboard/webapp/metrics/store/metrics_selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,47 +257,57 @@ export const getCanCreateNewPins = createSelector(
}
);

const selectSettings = createSelector(
selectMetricsState,
(state): MetricsState['settings'] => state.settings
);

/**
* Settings.
*/
export const getMetricsTooltipSort = createSelector(
selectMetricsState,
(state): TooltipSort => state.settings.tooltipSort
selectSettings,
(settings): TooltipSort => settings.tooltipSort
);

export const getMetricsIgnoreOutliers = createSelector(
selectMetricsState,
(state): boolean => state.settings.ignoreOutliers
selectSettings,
(settings): boolean => settings.ignoreOutliers
);

export const getMetricsXAxisType = createSelector(
selectMetricsState,
(state): XAxisType => state.settings.xAxisType
selectSettings,
(settings): XAxisType => settings.xAxisType
);

export const getMetricsHistogramMode = createSelector(
selectMetricsState,
(state): HistogramMode => state.settings.histogramMode
selectSettings,
(settings): HistogramMode => settings.histogramMode
);

export const getMetricsScalarSmoothing = createSelector(
selectMetricsState,
(state): number => state.settings.scalarSmoothing
selectSettings,
(settings): number => settings.scalarSmoothing
);

export const getMetricsScalarPartitionNonMonotonicX = createSelector(
selectSettings,
(settings): boolean => settings.scalarPartitionNonMonotonicX
);

export const getMetricsImageBrightnessInMilli = createSelector(
selectMetricsState,
(state): number => state.settings.imageBrightnessInMilli
selectSettings,
(settings): number => settings.imageBrightnessInMilli
);

export const getMetricsImageContrastInMilli = createSelector(
selectMetricsState,
(state): number => state.settings.imageContrastInMilli
selectSettings,
(settings): number => settings.imageContrastInMilli
);

export const getMetricsImageShowActualSize = createSelector(
selectMetricsState,
(state): boolean => state.settings.imageShowActualSize
selectSettings,
(settings): boolean => settings.imageShowActualSize
);

export const getMetricsTagFilter = createSelector(
Expand Down
14 changes: 14 additions & 0 deletions tensorboard/webapp/metrics/store/metrics_selectors_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,20 @@ describe('metrics selectors', () => {
expect(selectors.getMetricsScalarSmoothing(state)).toBe(0);
});

it('returns scalarPartitionNonMonotonicX', () => {
selectors.getMetricsScalarPartitionNonMonotonicX.release();
const state = appStateFromMetricsState(
buildMetricsState({
settings: buildMetricsSettingsState({
scalarPartitionNonMonotonicX: false,
}),
})
);
expect(selectors.getMetricsScalarPartitionNonMonotonicX(state)).toBe(
false
);
});

it('returns imageBrightnessInMilli when called getMetricsImageBrightnessInMilli', () => {
selectors.getMetricsImageBrightnessInMilli.release();
const state = appStateFromMetricsState(
Expand Down
11 changes: 11 additions & 0 deletions tensorboard/webapp/metrics/store/metrics_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,16 @@ export interface MetricsRoutelessState {
ignoreOutliers: boolean;
xAxisType: XAxisType;
scalarSmoothing: number;
/**
* https://github.com/tensorflow/tensorboard/issues/3732
*
* When a ML job restarts from a checkpoint or if a user writes to the same logdir
* with overlapping steps, TensorBoard shows a zig-zag lines which tend to confuse
* users. This setting guarantees that each line forms a monotonic increases in x-axis
* by creating a pseudo-runs by partitioning the runs on the client side. In the
* future, we may fix this at the log writing, reading, or backend response time.
*/
scalarPartitionNonMonotonicX: boolean;
/**
* A non-negative, unitless number. A value of 5000 corresponds to 500%
* increased brightness from normal.
Expand Down Expand Up @@ -190,6 +200,7 @@ export const METRICS_SETTINGS_DEFAULT: MetricsState['settings'] = {
ignoreOutliers: true,
xAxisType: XAxisType.STEP,
scalarSmoothing: 0.6,
scalarPartitionNonMonotonicX: false,
imageBrightnessInMilli: 1000,
imageContrastInMilli: 1000,
imageShowActualSize: false,
Expand Down
1 change: 1 addition & 0 deletions tensorboard/webapp/metrics/testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export function buildMetricsSettingsState(
ignoreOutliers: false,
xAxisType: XAxisType.WALL_TIME,
scalarSmoothing: 0.3,
scalarPartitionNonMonotonicX: false,
imageBrightnessInMilli: 123,
imageContrastInMilli: 123,
imageShowActualSize: true,
Expand Down
24 changes: 24 additions & 0 deletions tensorboard/webapp/metrics/views/right_pane/right_pane_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ describe('metrics right_pane', () => {
store.overrideSelector(selectors.getMetricsIgnoreOutliers, false);
store.overrideSelector(selectors.getMetricsXAxisType, XAxisType.STEP);
store.overrideSelector(selectors.getMetricsScalarSmoothing, 0.2);
store.overrideSelector(
selectors.getMetricsScalarPartitionNonMonotonicX,
false
);
store.overrideSelector(selectors.getMetricsImageBrightnessInMilli, 200);
store.overrideSelector(selectors.getMetricsImageContrastInMilli, 10);
store.overrideSelector(selectors.getMetricsImageShowActualSize, false);
Expand Down Expand Up @@ -105,6 +109,10 @@ describe('metrics right_pane', () => {
store.overrideSelector(selectors.getMetricsIgnoreOutliers, false);
store.overrideSelector(selectors.getMetricsXAxisType, XAxisType.STEP);
store.overrideSelector(selectors.getMetricsScalarSmoothing, 0.3);
store.overrideSelector(
selectors.getMetricsScalarPartitionNonMonotonicX,
true
);
store.overrideSelector(selectors.getMetricsImageBrightnessInMilli, 100);
store.overrideSelector(selectors.getMetricsImageContrastInMilli, 200);
store.overrideSelector(selectors.getMetricsImageShowActualSize, true);
Expand All @@ -125,6 +133,10 @@ describe('metrics right_pane', () => {
]
).toBe('false');

expect(
select(fixture, '.scalars-partition-x input').attributes['aria-checked']
).toBe('true');

const xAxisTypeSelect = select(fixture, '.x-axis-type tb-dropdown');
expect(xAxisTypeSelect.componentInstance.value).toBe(XAxisType.STEP);

Expand Down Expand Up @@ -224,6 +236,18 @@ describe('metrics right_pane', () => {
expect(dispatchSpy).not.toHaveBeenCalled();
}));

it('dispatches metricsScalarPartitionNonMonotonicXToggled on toggle', () => {
const fixture = TestBed.createComponent(SettingsViewContainer);
fixture.detectChanges();

const checkbox = select(fixture, '.scalars-partition-x input');
checkbox.nativeElement.click();

expect(dispatchSpy).toHaveBeenCalledWith(
actions.metricsScalarPartitionNonMonotonicXToggled()
);
});

it('dispatches metricsToggleIgnoreOutliers on toggle', () => {
const fixture = TestBed.createComponent(SettingsViewContainer);
fixture.detectChanges();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,21 @@ <h3 class="section-title">Scalars</h3>
>Ignore outliers in chart scaling</mat-checkbox
>
</div>

<div class="control-row scalars-partition-x">
<mat-checkbox
[checked]="scalarPartitionX"
(change)="scalarPartitionXToggled.emit()"
>Partition non-monotonic X axis</mat-checkbox
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I'm struggling to come up with better phrasing, but I'm wondering if we might be able to get away with slightly a longer visible label that explains how the partition will occur, without always having to read the full tooltip. Maybe something like:

"Partition non-monotonic time series into multiple monotonic series"
?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will keep the terse phrasing at least for now.

>
<mat-icon
class="info"
svgIcon="help_outline_24px"
title="Non-monotonic steps can occur when reusing a logdir with multiple summary writers and overlapping steps. Line charts, without this option enabled, can appear zig zagged. This is common when restarting from a checkpoint.

When enabled, a non-monotonic time series composed of N monotonic pieces will be shown as N monotonic lines."
></mat-icon>
</div>
</section>

<section class="Histograms">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,18 @@ section .control-row:not(:last-child) {
width: 5em;
}

.scalars-partition-x {
align-items: center;
display: flex;

.info {
$_dim: 15px;
height: $_dim;
margin-left: 5px;
width: $_dim;
}
}

mat-slider {
flex: 1;
// Reset mat-slider's internal extra space on left/right sides
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ export class SettingsViewComponent {
auditTime(SLIDER_AUDIT_TIME_MS)
);

@Input() scalarPartitionX!: boolean;
@Output() scalarPartitionXToggled = new EventEmitter();

onScalarSmoothingInput(event: Event) {
const input = event.target as HTMLInputElement;
if (!input.value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
metricsChangeXAxisType,
metricsResetImageBrightness,
metricsResetImageContrast,
metricsScalarPartitionNonMonotonicXToggled,
metricsToggleIgnoreOutliers,
metricsToggleImageShowActualSize,
} from '../../actions';
Expand All @@ -47,6 +48,8 @@ import {HistogramMode, TooltipSort, XAxisType} from '../../types';
(histogramModeChanged)="onHistogramModeChanged($event)"
[scalarSmoothing]="scalarSmoothing$ | async"
(scalarSmoothingChanged)="onScalarSmoothingChanged($event)"
[scalarPartitionX]="scalarPartitionX$ | async"
(scalarPartitionXToggled)="onScalarPartitionXToggled()"
[imageBrightnessInMilli]="imageBrightnessInMilli$ | async"
(imageBrightnessInMilliChanged)="onImageBrightnessInMilliChanged($event)"
(imageBrightnessReset)="onImageBrightnessReset()"
Expand Down Expand Up @@ -74,6 +77,9 @@ export class SettingsViewContainer {
readonly scalarSmoothing$ = this.store.select(
selectors.getMetricsScalarSmoothing
);
readonly scalarPartitionX$ = this.store.select(
selectors.getMetricsScalarPartitionNonMonotonicX
);
readonly imageBrightnessInMilli$ = this.store.select(
selectors.getMetricsImageBrightnessInMilli
);
Expand Down Expand Up @@ -104,6 +110,10 @@ export class SettingsViewContainer {
this.store.dispatch(metricsChangeScalarSmoothing({smoothing}));
}

onScalarPartitionXToggled() {
this.store.dispatch(metricsScalarPartitionNonMonotonicXToggled());
}

onImageBrightnessInMilliChanged(brightnessInMilli: number) {
this.store.dispatch(metricsChangeImageBrightness({brightnessInMilli}));
}
Expand Down