Skip to content

Commit 05a891a

Browse files
committed
Improve the scalar chart user experience
Summary: This fixes a number of issues with the scalar charts, some of which are interdependent, and all of which are existing issues made more visible by the presence of a search pane. In particular: 1. If a chart's `run` and `tag` properties are changed simultaneously, two updates will be triggered, and the first update will see the data in an inconsistent state (e.g., Tag B but still Run A). We fix the resulting issues by making observer updates asynchronous, so that they only actually take effect once all changes have flushed. (This fixes #169.) 2. To reduce latency due to the above solution, we further debounce these update handlers. 3. Because the handlers are asynchronous and debounced, we need to maintain stronger invariants on the caching keys. (In particular, if a chart changes from Tag A to Tag B and then to Tag C, we don't want the network requests for Tag B to fill the runs-loaded cache and then have Tag C forgo its fetches.) 4. Furthermore, when the tag of a chart is changed, we display a loading indicator. This is to prevent problems in the scenario where many charts change from one tag to another: in this case, all the charts' titles will update immediately (they're just bound to text nodes in the DOM), but charts' data will take a while to update; the result is that there are many charts with titles that simply do not match their data, which is at best very confusing. The loading indicator makes it clear that the data is stale. 5. When a chart's tag is changed, we now reset the domain of the chart once all the data has loaded. (This fixes #164.) 6. Finally, we offer a button to manually reset the domain of the chart. (This is often useful when changing the set of selected runs: after adding a new run, you may need to expand the domain, but we don't do this automatically because you may often want to keep your old domain.) Issues (1), (4), and (5) arise frequently when changing a search query, as this causes many charts to change their tag and runs. Issues (1), (2), and (4) apply to other dashboards as well, but have not been fixed. (Issue (3) would apply to other dashboards that display one visualization per tag instead of per run-tag combination, but there are no such dashboards.) Test Plan: Cherry-pick #173 before proceeding. (Without a search feature, the effects of this change are minimal---but I want to merge this change before I merge the search feature!) Launch TensorBoard with a few dozen scalar tags. Suppose without loss of generality that the name of the fifth tag displayed in the search results pane (on query `.*`) is `foo`. Set the search query to `(?!foo)`; this should still match all tags. Now, add a caret at the beginning to search for `^(?!foo)`. Charts past the fifth one should reload. Check that the reloading behavior is pleasant. Then, zoom into some charts, manipulating their windows. Click the "reload" button, or wait 30 seconds, and ensure that the window is preserved. Change the search query, though, and the windows should be reset to show all the data. wchargin-branch: improve-scalars-ux
1 parent 47ba0c8 commit 05a891a

File tree

3 files changed

+153
-32
lines changed

3 files changed

+153
-32
lines changed

tensorboard/plugins/scalar/tf_scalar_dashboard/BUILD

+1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ ts_web_library(
3333
"@org_polymer_paper_item",
3434
"@org_polymer_paper_menu",
3535
"@org_polymer_paper_slider",
36+
"@org_polymer_paper_spinner",
3637
"@org_polymer_paper_styles",
3738
],
3839
)

tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-chart.html

+126-31
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
<link rel="import" href="../paper-icon-button/paper-icon-button.html">
2020
<link rel="import" href="../paper-item/paper-item.html">
2121
<link rel="import" href="../paper-menu/paper-menu.html">
22+
<link rel="import" href="../paper-spinner/paper-spinner-lite.html">
2223
<link rel="import" href="../polymer/polymer.html">
2324
<link rel="import" href="../tf-backend/tf-backend.html">
2425
<link rel="import" href="../tf-card-heading/tf-card-heading.html">
@@ -32,14 +33,22 @@
3233
<dom-module id="tf-scalar-chart">
3334
<template>
3435
<tf-card-heading title="[[tag]]"></tf-card-heading>
35-
<vz-line-chart
36-
x-type="[[xType]]"
37-
color-scale="[[_runsColorScale]]"
38-
smoothing-enabled="[[smoothingEnabled]]"
39-
smoothing-weight="[[smoothingWeight]]"
40-
tooltip-sorting-method="[[tooltipSortingMethod]]"
41-
ignore-y-outliers="[[ignoreYOutliers]]"
42-
></vz-line-chart>
36+
<div id="chart-and-spinner-container">
37+
<vz-line-chart
38+
x-type="[[xType]]"
39+
color-scale="[[_runsColorScale]]"
40+
smoothing-enabled="[[smoothingEnabled]]"
41+
smoothing-weight="[[smoothingWeight]]"
42+
tooltip-sorting-method="[[tooltipSortingMethod]]"
43+
ignore-y-outliers="[[ignoreYOutliers]]"
44+
style="[[_computeLineChartStyle(_loading)]]"
45+
></vz-line-chart>
46+
<template is="dom-if" if="[[_loading]]">
47+
<div id="loading-spinner-container">
48+
<paper-spinner-lite active></paper-spinner-lite>
49+
</div>
50+
</template>
51+
</div>
4352
<div style="display: flex; flex-direction: row;">
4453
<paper-icon-button
4554
selected$="[[_expanded]]"
@@ -52,6 +61,11 @@
5261
on-tap="_toggleLogScale"
5362
title="Toggle y-axis log scale"
5463
></paper-icon-button>
64+
<paper-icon-button
65+
icon="settings-overscan"
66+
on-tap="_resetDomain"
67+
title="Fit domain to data"
68+
></paper-icon-button>
5569
<span style="flex-grow: 1"></span>
5670
<template is="dom-if" if="[[showDownloadLinks]]">
5771
<div class="download-links">
@@ -89,6 +103,23 @@
89103
height: 400px;
90104
width: 100%;
91105
}
106+
#chart-and-spinner-container {
107+
display: flex;
108+
flex-grow: 1;
109+
position: relative;
110+
}
111+
#loading-spinner-container {
112+
align-items: center;
113+
bottom: 0;
114+
display: flex;
115+
display: flex;
116+
justify-content: center;
117+
left: 0;
118+
pointer-events: none;
119+
position: absolute;
120+
right: 0;
121+
top: 0;
122+
}
92123
vz-line-chart {
93124
-webkit-user-select: none;
94125
-moz-user-select: none;
@@ -167,6 +198,16 @@
167198
value: () => ({scale: runsColorScale}),
168199
},
169200

201+
_loading: {
202+
type: Boolean,
203+
value: false,
204+
},
205+
206+
_resetDomainOnNextLoad: {
207+
type: Boolean,
208+
value: true,
209+
},
210+
170211
_canceller: {
171212
type: Object,
172213
value: () => new Canceller(),
@@ -199,9 +240,13 @@
199240
},
200241
},
201242
observers: [
202-
'reload(tag)',
203-
'_changeSeries(runs.*)'
243+
'_tagChanged(_attached, tag)',
244+
'_runsChanged(_attached, runs.*)'
204245
],
246+
created() {
247+
this._loadData = _.debounce(
248+
this._loadData, 100, {leading: true, trailing: true});
249+
},
205250
attached() {
206251
this._attached = true;
207252
this._changeSeries();
@@ -210,33 +255,72 @@
210255
this._loadedRuns = {};
211256
this._loadData();
212257
},
213-
_loadData() {
214-
if (!this._attached) {
258+
_tagChanged(attached, tagUpdateRecord) {
259+
this._loadedRuns = {};
260+
this._resetDomainOnNextLoad = true;
261+
this._loadData();
262+
},
263+
_runsChanged(attached, runsUpdateRecord) {
264+
if (!attached) {
215265
return;
216266
}
217-
//
218-
// Before updating, cancel any network-pending updates, to
219-
// prevent race conditions where older data stomps newer data.
220-
this._canceller.cancelAll();
221-
this.runs.forEach(run => {
222-
if (this._loadedRuns[run]) {
267+
this.$$('vz-line-chart').setVisibleSeries(this.runs);
268+
this._loadData();
269+
},
270+
_loadData() {
271+
this.async(() => {
272+
if (!this._attached) {
223273
return;
224274
}
225-
this._loadedRuns[run] = true;
226-
const url = this._scalarUrl(this.tag, run);
227-
const updateSeries = this._canceller.cancellable(result => {
228-
if (result.cancelled) {
229-
return;
275+
this._loading = true;
276+
//
277+
// Before updating, cancel any network-pending updates, to
278+
// prevent race conditions where older data stomps newer data.
279+
this._canceller.cancelAll();
280+
const tag = this.tag;
281+
const runPromises = this.runs.map(run => {
282+
if (this._loadedRuns[run]) {
283+
return Promise.resolve();
284+
}
285+
const url = this._scalarUrl(tag, run);
286+
const updateSeries = this._canceller.cancellable(result => {
287+
if (result.cancelled) {
288+
return;
289+
}
290+
const data = result.value;
291+
const formattedData = data.map(datum => ({
292+
wall_time: new Date(datum[0] * 1000),
293+
step: datum[1],
294+
scalar: datum[2],
295+
}));
296+
if (tag === this.tag) {
297+
// Only update the runs cache for the current tag. If we
298+
// load data for Tag A, then the tag changes to Tag B
299+
// while requests are still in flight, these requests
300+
// should not poison the cache.
301+
this._loadedRuns[run] = true;
302+
}
303+
this.$$('vz-line-chart').setSeriesData(run, formattedData);
304+
});
305+
return this.requestManager.request(url).then(updateSeries);
306+
});
307+
const finish = this._canceller.cancellable(result => {
308+
if (!result.cancelled) {
309+
this._loading = false;
310+
const chart = this.$$('vz-line-chart');
311+
if (runPromises.length > 0 && this._resetDomainOnNextLoad) {
312+
// (Don't unset _resetDomainOnNextLoad when we didn't
313+
// load any runs: this has the effect that if all our
314+
// runs are deselected, then we toggle them all on, we
315+
// properly size the domain once all the data is loaded
316+
// instead of just when we're first rendered.)
317+
this._resetDomainOnNextLoad = false;
318+
chart.resetDomain();
319+
}
320+
chart.redraw();
230321
}
231-
const data = result.value;
232-
const formattedData = data.map(datum => ({
233-
wall_time: new Date(datum[0] * 1000),
234-
step: datum[1],
235-
scalar: datum[2],
236-
}));
237-
this.$$('vz-line-chart').setSeriesData(run, formattedData);
238322
});
239-
this.requestManager.request(url).then(updateSeries);
323+
return Promise.all(runPromises).then(finish);
240324
});
241325
},
242326
_changeSeries() {
@@ -261,6 +345,17 @@
261345
this.redraw();
262346
},
263347

348+
_computeLineChartStyle(loading) {
349+
return loading ? 'opacity: 0.3;' : '';
350+
},
351+
352+
_resetDomain() {
353+
const chart = this.$$('vz-line-chart');
354+
if (chart) {
355+
chart.resetDomain();
356+
}
357+
},
358+
264359
_csvUrl(run) {
265360
return this._scalarUrl(this.tag, run) + '&format=csv';
266361
},

tensorboard/plugins/scalar/vz_line_chart/vz-line-chart.ts

+26-1
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,22 @@ Polymer({
167167
}
168168
},
169169

170+
/**
171+
* Reset the chart domain to fit its data.
172+
*/
173+
resetDomain: function() {
174+
if (this._chart) {
175+
this._chart.resetDomain();
176+
}
177+
},
178+
170179
/**
171180
* Re-renders the chart. Useful if e.g. the container size changed.
172181
*/
173182
redraw: function() {
174-
this._chart.redraw();
183+
if (this._chart) {
184+
this._chart.redraw();
185+
}
175186
},
176187
attached: function() {
177188
this._attached = true;
@@ -449,6 +460,20 @@ class LineChart {
449460
this.nanDataset.data(nanData);
450461
}
451462

463+
public resetDomain() {
464+
this.resetXDomain();
465+
this.resetYDomain();
466+
}
467+
468+
private resetXDomain() {
469+
// (Copied from DragZoomLayer.unzoom.)
470+
const xScale = this.xScale as any;
471+
xScale._domainMin = null;
472+
xScale._domainMax = null;
473+
const xDomain = xScale._getExtent();
474+
this.xScale.domain(xDomain);
475+
}
476+
452477
private resetYDomain() {
453478
const accessor = this.getAccessor();
454479
let datasetToValues: (d: Plottable.Dataset) => number[] = (d) => {

0 commit comments

Comments
 (0)