Skip to content

Commit 920dd1f

Browse files
authored
hparams: fix the parallel coordinate layout (#4988)
Bug --- Previously, we were setting the width of the container based on count of all hparams and metrics without considering the visibility. This had caused the UI to be in very awkward state where each visible hparams and metrics are spaced a lot even if the selection count is quite low. Fix --- We layout the parallel coordinates based on number of visible {hparam,metric}s. The change is a bit larger because: - I had to add type information and refactor it a little for understanding the code. - Undo the performance optimization where we do not mutate the layout unless important piece of information in the configuration has changed. To detect the old configuration vs. new configuration, we have to now remember an additional state in the component. Fixes #3414.
1 parent 9021d2a commit 920dd1f

File tree

8 files changed

+70
-26
lines changed

8 files changed

+70
-26
lines changed

tensorboard/plugins/hparams/tf_hparams_parallel_coords_plot/BUILD

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ tf_ts_library(
1717
deps = [
1818
"//tensorboard/components/polymer:legacy_element_mixin",
1919
"//tensorboard/plugins/hparams:types",
20-
"//tensorboard/plugins/hparams/tf_hparams_query_pane:schema_d_ts",
2120
"//tensorboard/plugins/hparams/tf_hparams_session_group_values",
21+
"//tensorboard/plugins/hparams/tf_hparams_types",
2222
"//tensorboard/plugins/hparams/tf_hparams_utils",
2323
"@npm//@polymer/decorators",
2424
"@npm//@polymer/polymer",

tensorboard/plugins/hparams/tf_hparams_parallel_coords_plot/axes.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import * as d3 from 'd3';
1818
import * as tf_hparams_parallel_coords_plot_interaction_manager from './interaction_manager';
1919
import * as tf_hparams_utils from '../tf_hparams_utils/tf-hparams-utils';
2020
import * as tf_hparams_parallel_coords_plot_utils from './utils';
21-
import * as tf_hparams_query_pane from '../tf_hparams_query_pane/schema.d';
21+
import {Schema} from '../tf_hparams_types/types';
2222
import * as tf_hparams_api from '../types';
2323

2424
export enum ScaleType {
@@ -120,7 +120,7 @@ export class Axis {
120120
*/
121121
public constructor(
122122
svgProps: tf_hparams_parallel_coords_plot_interaction_manager.SVGProperties,
123-
schema: tf_hparams_query_pane.Schema,
123+
schema: Schema,
124124
interactionManager: tf_hparams_parallel_coords_plot_interaction_manager.InteractionManager,
125125
colIndex: number
126126
) {
@@ -362,7 +362,7 @@ export class Axis {
362362
return new AlwaysPassingBrushFilter();
363363
}
364364
private readonly _svgProps: tf_hparams_parallel_coords_plot_interaction_manager.SVGProperties;
365-
private readonly _schema: tf_hparams_query_pane.Schema;
365+
private readonly _schema: Schema;
366366
private readonly _interactionManager: tf_hparams_parallel_coords_plot_interaction_manager.InteractionManager;
367367
private readonly _colIndex: number;
368368
private _isDisplayed: boolean;
@@ -379,7 +379,7 @@ export class Axis {
379379
export class AxesCollection {
380380
public constructor(
381381
svgProps: tf_hparams_parallel_coords_plot_interaction_manager.SVGProperties,
382-
schema: tf_hparams_query_pane.Schema,
382+
schema: Schema,
383383
interactionManager: tf_hparams_parallel_coords_plot_interaction_manager.InteractionManager
384384
) {
385385
this._svgProps = svgProps;
@@ -556,7 +556,7 @@ export class AxesCollection {
556556
);
557557
}
558558
private _svgProps: tf_hparams_parallel_coords_plot_interaction_manager.SVGProperties;
559-
private _schema: tf_hparams_query_pane.Schema;
559+
private _schema: Schema;
560560
private _axes: Axis[];
561561
/**
562562
* The current assignment of stationary positions to axes.

tensorboard/plugins/hparams/tf_hparams_parallel_coords_plot/interaction_manager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import * as d3 from 'd3';
2020
import * as tf_hparams_utils from '../tf_hparams_utils/tf-hparams-utils';
2121
import {AxesCollection} from './axes';
2222
import {LinesCollection, LineType, SessionGroupHandle} from './lines';
23-
import * as tf_hparams_query_pane from '../tf_hparams_query_pane/schema.d';
23+
import * as tf_hparams_query_pane from '../tf_hparams_types/types';
2424

2525
import * as tf_hparams_api from '../types';
2626

tensorboard/plugins/hparams/tf_hparams_parallel_coords_plot/lines.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import * as tf_hparams_parallel_coords_plot_utils from './utils';
2222
import {AxesCollection} from './axes';
2323
import * as tf_hparams_parallel_coords_plot_interaction_manager from './interaction_manager';
2424

25-
import * as tf_hparams_query_pane from '../tf_hparams_query_pane/schema.d';
25+
import * as tf_hparams_query_pane from '../tf_hparams_types/types';
2626

2727
import * as tf_hparams_api from '../types';
2828

tensorboard/plugins/hparams/tf_hparams_parallel_coords_plot/tf-hparams-parallel-coords-plot.ts

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,27 @@ limitations under the License.
6060
*
6161
* See the individual class comments in the respective files for more details.
6262
*/
63-
import {PolymerElement, html} from '@polymer/polymer';
6463
import {customElement, observe, property} from '@polymer/decorators';
65-
import * as _ from 'lodash';
64+
import {html, PolymerElement} from '@polymer/polymer';
6665
import * as d3 from 'd3';
66+
import * as _ from 'lodash';
6767

68-
import * as tf_hparams_utils from '../tf_hparams_utils/tf-hparams-utils';
68+
import {LegacyElementMixin} from '../../../components/polymer/legacy_element_mixin';
6969
import '../tf_hparams_session_group_values/tf-hparams-session-group-values';
70+
import {HparamInfo, MetricInfo, Schema} from '../tf_hparams_types/types';
71+
import * as tf_hparams_utils from '../tf_hparams_utils/tf-hparams-utils';
7072
import * as tf_hparams_parallel_coords_plot_interaction_manager from './interaction_manager';
7173

72-
import {LegacyElementMixin} from '../../../components/polymer/legacy_element_mixin';
74+
interface Option {
75+
configuration: {
76+
columnsVisibility: boolean[];
77+
schema: Schema;
78+
visibleSchema: {
79+
hparamInfos: HparamInfo[];
80+
metricInfos: MetricInfo[];
81+
};
82+
};
83+
}
7384

7485
@customElement('tf-hparams-parallel-coords-plot')
7586
class TfHparamsParallelCoordsPlot extends LegacyElementMixin(PolymerElement) {
@@ -136,9 +147,13 @@ class TfHparamsParallelCoordsPlot extends LegacyElementMixin(PolymerElement) {
136147
// See the property description in tf-hparams-query-pane.html
137148
@property({type: Array})
138149
sessionGroups: any[];
150+
139151
// See the description in tf-hparams-scale-and-color-controls.html
140152
@property({type: Object})
141-
options: any;
153+
options: Option;
154+
155+
private _prevOptions?: Option;
156+
142157
// The last session group that was clicked on or null if no
143158
// session group was clicked on yet.
144159
/**
@@ -179,33 +194,37 @@ class TfHparamsParallelCoordsPlot extends LegacyElementMixin(PolymerElement) {
179194
// element. Defined in tf-hparams-parallel-coords-plot.ts.
180195
@property({type: Object})
181196
_interactionManager: any;
197+
182198
@observe('options.*', 'sessionGroups.*')
183199
_optionsOrSessionGroupsChanged() {
184200
if (!this.options) {
185201
return;
186202
}
187-
const configuration = this.options.configuration;
203+
204+
const {configuration: prevConfig} = this._prevOptions ?? {};
205+
const {configuration: nextConfig} = this.options;
188206
// See if we need to redraw from scratch. We redraw from scratch if
189207
// this is initialization or if configuration.schema has changed.
190208
if (
191209
this._interactionManager === undefined ||
192-
!_.isEqual(this._interactionManager.schema(), configuration.schema)
210+
!_.isEqual(prevConfig.schema, nextConfig.schema) ||
211+
!_.isEqual(prevConfig.columnsVisibility, nextConfig.columnsVisibility)
193212
) {
194213
// Remove any pre-existing DOM children of our SVG.
195214
d3.select(this.$.svg as SVGElement)
196215
.selectAll('*')
197216
.remove();
198217
const svgProps = new tf_hparams_parallel_coords_plot_interaction_manager.SVGProperties(
199218
this.$.svg as HTMLElement,
200-
tf_hparams_utils.numColumns(configuration.schema)
219+
nextConfig.columnsVisibility.filter(Boolean).length
201220
);
202221
// Listen to DOM changes underneath this.$.svg, and apply local CSS
203222
// scoping rules so that our rules in the <style> section above
204223
// would apply.
205224
this.scopeSubtree(this.$.svg as SVGElement, true);
206225
this._interactionManager = new tf_hparams_parallel_coords_plot_interaction_manager.InteractionManager(
207226
svgProps,
208-
configuration.schema,
227+
nextConfig.schema,
209228
(sessionGroup) => this.closestSessionGroupChanged(sessionGroup),
210229
(sessionGroup) => this.selectedSessionGroupChanged(sessionGroup)
211230
);
@@ -216,6 +235,7 @@ class TfHparamsParallelCoordsPlot extends LegacyElementMixin(PolymerElement) {
216235
this._validSessionGroups
217236
);
218237
this.redrawCount++;
238+
this._prevOptions = this.options;
219239
}
220240
closestSessionGroupChanged(sessionGroup) {
221241
this.closestSessionGroup = sessionGroup;

tensorboard/plugins/hparams/tf_hparams_query_pane/BUILD

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,6 @@ package(default_visibility =
55

66
licenses(["notice"])
77

8-
tf_ts_library(
9-
name = "schema_d_ts",
10-
srcs = [
11-
"schema.d.ts",
12-
],
13-
)
14-
158
tf_ts_library(
169
name = "tf_hparams_query_pane",
1710
srcs = [
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
load("//tensorboard/defs:defs.bzl", "tf_ts_library")
2+
3+
package(default_visibility = ["//tensorboard/plugins/hparams:__subpackages__"])
4+
5+
licenses(["notice"])
6+
7+
tf_ts_library(
8+
name = "tf_hparams_types",
9+
srcs = [
10+
"types.d.ts",
11+
],
12+
)

tensorboard/plugins/hparams/tf_hparams_query_pane/schema.d.ts renamed to tensorboard/plugins/hparams/tf_hparams_types/types.d.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,30 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
See the License for the specific language governing permissions and
1313
limitations under the License.
1414
==============================================================================*/
15+
16+
export interface HparamInfo {
17+
description: string;
18+
displayName: string;
19+
name: string;
20+
type:
21+
| 'DATA_TYPE_UNSET'
22+
| 'DATA_TYPE_STRING'
23+
| 'DATA_TYPE_BOOL'
24+
| 'DATA_TYPE_FLOAT64';
25+
}
26+
27+
export interface MetricInfo {
28+
datasetType: 'DATASET_UNKNOWN' | 'DATASET_TRAINING' | 'DATASET_VALIDATION';
29+
description: string;
30+
displayName: string;
31+
name: {tag: string; group: string};
32+
}
33+
1534
export interface Schema {
1635
hparamColumn: Array<{
17-
hparamInfo: Object;
36+
hparamInfo: HparamInfo;
1837
}>;
1938
metricColumn: Array<{
20-
metricInfo: Object;
39+
metricInfo: MetricInfo;
2140
}>;
2241
}

0 commit comments

Comments
 (0)