Skip to content

Commit d8b7a9d

Browse files
authored
Data Table: Sort Run Names Without Id and Alias (#5987)
* Motivation for features / changes For Googlers [b/254089050](b/254089050) Run names are currently sorted differently in the data table than in the tool tip. I'm changing the sorting to be consistent with the tooltips alphabetical sort. * Screenshots of UI changes ![51842c87-6bf0-4492-8c93-34535f845943](https://user-images.githubusercontent.com/78179109/196759166-be218ef4-1aaf-4383-9d61-fc6395a3db9a.gif) * Detailed steps to verify changes work correctly (as executed by you) 1) Start tensorboard 2) Visit localhost:6006?enableDataTable&enableRangeSelection 3) Click the "Enable Range Selection" checkbox in the settings panel 4) Sort the scalar card data table 5) Assert that the table is sorted by the run name only (not the alias or alias id) * Alternate designs / implementations considered I considered creating a column alias map but this approach seemed more flexible.
1 parent 457a0ab commit d8b7a9d

File tree

4 files changed

+113
-6
lines changed

4 files changed

+113
-6
lines changed

tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,9 @@ export class ScalarCardDataTable {
104104
closestEndPointIndex = findClosestIndex(datum.points, endStep);
105105
closestEndPoint = datum.points[closestEndPointIndex];
106106
}
107-
const selectedStepData: SelectedStepRunData = {};
107+
const selectedStepData: SelectedStepRunData = {
108+
id: datum.id,
109+
};
108110
selectedStepData.COLOR = metadata.color;
109111
for (const header of this.dataHeaders) {
110112
switch (header) {
@@ -189,8 +191,8 @@ export class ScalarCardDataTable {
189191
});
190192
dataTableData.sort(
191193
(point1: SelectedStepRunData, point2: SelectedStepRunData) => {
192-
const p1 = getSortableValue(point1[this.sortingInfo.header]);
193-
const p2 = getSortableValue(point2[this.sortingInfo.header]);
194+
const p1 = this.getSortableValue(point1, this.sortingInfo.header);
195+
const p2 = this.getSortableValue(point2, this.sortingInfo.header);
194196
if (p1 < p2) {
195197
return this.sortingInfo.order === SortingOrder.ASCENDING ? -1 : 1;
196198
}
@@ -203,9 +205,20 @@ export class ScalarCardDataTable {
203205
);
204206
return dataTableData;
205207
}
208+
209+
private getSortableValue(point: SelectedStepRunData, header: ColumnHeaders) {
210+
switch (header) {
211+
// The value shown in the "RUN" column is a string concatenation of Alias Id + Alias + Run Name
212+
// but we would actually prefer to sort by just the run name.
213+
case ColumnHeaders.RUN:
214+
return makeValueSortable(this.chartMetadataMap[point.id].displayName);
215+
default:
216+
return makeValueSortable(point[header]);
217+
}
218+
}
206219
}
207220

208-
function getSortableValue(value: number | string | undefined | null) {
221+
function makeValueSortable(value: number | string | null | undefined) {
209222
if (
210223
Number.isNaN(value) ||
211224
value === 'NaN' ||

tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2541,13 +2541,15 @@ describe('scalar card', () => {
25412541

25422542
expect(data).toEqual([
25432543
{
2544+
id: 'run1',
25442545
COLOR: '#fff',
25452546
RELATIVE_TIME: 1000,
25462547
RUN: 'run1',
25472548
STEP: 2,
25482549
VALUE: 10,
25492550
},
25502551
{
2552+
id: 'run2',
25512553
COLOR: '#fff',
25522554
RELATIVE_TIME: 1000,
25532555
RUN: 'run2',
@@ -2601,6 +2603,7 @@ describe('scalar card', () => {
26012603

26022604
expect(data).toEqual([
26032605
{
2606+
id: 'run1',
26042607
COLOR: '#fff',
26052608
RUN: 'run1',
26062609
VALUE_CHANGE: 19,
@@ -2613,6 +2616,7 @@ describe('scalar card', () => {
26132616
PERCENTAGE_CHANGE: 19, // percentage change from 1 to 20 is 1900%
26142617
},
26152618
{
2619+
id: 'run2',
26162620
COLOR: '#fff',
26172621
RUN: 'run2',
26182622
VALUE_CHANGE: 24,
@@ -3093,6 +3097,95 @@ describe('scalar card', () => {
30933097
expect(data[5].RUN).toEqual('run6');
30943098
expect(data[6].RUN).toEqual('run7');
30953099
}));
3100+
3101+
it('Sorts RUNS column by displayName', fakeAsync(() => {
3102+
const runToSeries = {
3103+
run1: [{wallTime: 1, value: 1, step: 1}],
3104+
run2: [{wallTime: 1, value: 2, step: 1}],
3105+
run3: [{wallTime: 1, value: 3, step: 1}],
3106+
run4: [{wallTime: 1, value: NaN, step: 1}],
3107+
run5: [{wallTime: 1, value: 'NaN', step: 1}],
3108+
run6: [{wallTime: 1, value: null, step: 1}],
3109+
run7: [{wallTime: 1, value: undefined, step: 1}],
3110+
};
3111+
provideMockCardRunToSeriesData(
3112+
selectSpy,
3113+
PluginType.SCALARS,
3114+
'card1',
3115+
null /* metadataOverride */,
3116+
runToSeries as any
3117+
);
3118+
store.overrideSelector(
3119+
selectors.getCurrentRouteRunSelection,
3120+
new Map([
3121+
['run1', true],
3122+
['run2', true],
3123+
['run3', true],
3124+
['run4', true],
3125+
['run5', true],
3126+
['run6', true],
3127+
['run7', true],
3128+
])
3129+
);
3130+
3131+
store.overrideSelector(getMetricsLinkedTimeSelection, {
3132+
start: {step: 1},
3133+
end: null,
3134+
});
3135+
3136+
const fixture = createComponent('card1');
3137+
const scalarCardDataTable = fixture.debugElement.query(
3138+
By.directive(ScalarCardDataTable)
3139+
);
3140+
scalarCardDataTable.componentInstance.sortingInfo = {
3141+
header: ColumnHeaders.RUN,
3142+
order: SortingOrder.ASCENDING,
3143+
};
3144+
scalarCardDataTable.componentInstance.chartMetadataMap.run1.alias = {
3145+
aliasText: 'g',
3146+
aliasNumber: 5,
3147+
};
3148+
scalarCardDataTable.componentInstance.chartMetadataMap.run2.alias = {
3149+
aliasText: 'f',
3150+
aliasNumber: 6,
3151+
};
3152+
scalarCardDataTable.componentInstance.chartMetadataMap.run3.alias = {
3153+
aliasText: 'e',
3154+
aliasNumber: 7,
3155+
};
3156+
scalarCardDataTable.componentInstance.chartMetadataMap.run4.alias = {
3157+
aliasText: 'd',
3158+
aliasNumber: 4,
3159+
};
3160+
scalarCardDataTable.componentInstance.chartMetadataMap.run5.alias = {
3161+
aliasText: 'b',
3162+
aliasNumber: 2,
3163+
};
3164+
scalarCardDataTable.componentInstance.chartMetadataMap.run6.alias = {
3165+
aliasText: 'c',
3166+
aliasNumber: 3,
3167+
};
3168+
scalarCardDataTable.componentInstance.chartMetadataMap.run7.alias = {
3169+
aliasText: 'a',
3170+
aliasNumber: 1,
3171+
};
3172+
console.log(
3173+
'ChartMetadatMap: ',
3174+
scalarCardDataTable.componentInstance.chartMetadataMap
3175+
);
3176+
fixture.detectChanges();
3177+
3178+
const data =
3179+
scalarCardDataTable.componentInstance.getTimeSelectionTableData();
3180+
3181+
expect(data[0].RUN).toEqual('5 g/run1');
3182+
expect(data[1].RUN).toEqual('6 f/run2');
3183+
expect(data[2].RUN).toEqual('7 e/run3');
3184+
expect(data[3].RUN).toEqual('4 d/run4');
3185+
expect(data[4].RUN).toEqual('2 b/run5');
3186+
expect(data[5].RUN).toEqual('3 c/run6');
3187+
expect(data[6].RUN).toEqual('1 a/run7');
3188+
}));
30963189
});
30973190

30983191
describe('step selector feature integration', () => {

tensorboard/webapp/metrics/views/card_renderer/scalar_card_types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ export interface SortingInfo {
111111
*/
112112
export type SelectedStepRunData = {
113113
[key in ColumnHeaders]?: string | number;
114-
};
114+
} & {id: string};
115115

116116
/**
117117
* An object which is intended to hold the min and max step within each scalar

tensorboard/webapp/widgets/data_table/data_table_test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ describe('data table', () => {
148148
],
149149
data: [
150150
{
151+
id: 'someid',
151152
RUN: 'run name',
152153
VALUE: 3,
153154
STEP: 1,
@@ -202,7 +203,7 @@ describe('data table', () => {
202203
ColumnHeaders.STEP,
203204
ColumnHeaders.RELATIVE_TIME,
204205
],
205-
data: [{}],
206+
data: [{id: 'someid'}],
206207
});
207208
fixture.detectChanges();
208209
const dataElements = fixture.debugElement.queryAll(By.css('td'));

0 commit comments

Comments
 (0)