Skip to content

Commit 9b76dbc

Browse files
rileyajonesbmd3k
authored andcommitted
Sort run names with leading numbers differently (tensorflow#6664)
## Motivation for features / changes We have been treating all run names as strings even though some run names are (serialized) numbers and some begin with numbers. This means that sorting worked pretty unintuitively. Note: I've also changed the behavior around sorting `undefined` values. When sorting descending they should now appear at the top of the list. This was a recent change but it wasn't clear it was intentional and I found it made the code more complex. tensorflow#6651 Internal users see [b/278671226](http://b/278671226) ## Screenshots of UI changes (or N/A) Before: ![image](https://github.com/tensorflow/tensorboard/assets/78179109/5f805c37-283d-4f55-b1bf-3dfa4d9ea1da) After: ![image](https://github.com/tensorflow/tensorboard/assets/78179109/0d740b6e-2ed5-4762-aec0-22400eeb152d) ![image](https://github.com/tensorflow/tensorboard/assets/78179109/5283bade-b4da-401d-9893-932d9fb9d378)
1 parent bd27895 commit 9b76dbc

File tree

4 files changed

+420
-29
lines changed

4 files changed

+420
-29
lines changed

tensorboard/webapp/runs/views/runs_table/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ tf_ng_module(
6464
"runs_table_component.ts",
6565
"runs_table_container.ts",
6666
"runs_table_module.ts",
67+
"sorting_utils.ts",
6768
],
6869
assets = [
6970
":regex_edit_dialog_styles",
@@ -131,6 +132,7 @@ tf_ts_library(
131132
"regex_edit_dialog_test.ts",
132133
"runs_data_table_test.ts",
133134
"runs_table_test.ts",
135+
"sorting_utils_test.ts",
134136
],
135137
deps = [
136138
":runs_table",

tensorboard/webapp/runs/views/runs_table/runs_table_container.ts

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ import {
7070
ColumnHeader,
7171
FilterAddedEvent,
7272
SortingInfo,
73-
SortingOrder,
7473
TableData,
7574
} from '../../../widgets/data_table/types';
7675
import {
@@ -101,6 +100,7 @@ import {
101100
getPotentialHparamColumns,
102101
} from '../../../metrics/views/main_view/common_selectors';
103102
import {runsTableFullScreenToggled} from '../../../core/actions';
103+
import {sortTableDataItems} from './sorting_utils';
104104

105105
const getRunsLoading = createSelector<
106106
State,
@@ -182,34 +182,6 @@ function sortRunTableItems(
182182
return sortedItems;
183183
}
184184

185-
function sortTableDataItems(
186-
items: TableData[],
187-
sort: SortingInfo
188-
): TableData[] {
189-
const sortedItems = [...items];
190-
191-
sortedItems.sort((a, b) => {
192-
let aValue = a[sort.name];
193-
let bValue = b[sort.name];
194-
195-
if (sort.name === 'experimentAlias') {
196-
aValue = (aValue as ExperimentAlias).aliasNumber;
197-
bValue = (bValue as ExperimentAlias).aliasNumber;
198-
}
199-
200-
if (aValue === bValue) {
201-
return 0;
202-
}
203-
204-
if (aValue === undefined || bValue === undefined) {
205-
return bValue === undefined ? -1 : 1;
206-
}
207-
208-
return aValue < bValue === (sort.order === SortingOrder.ASCENDING) ? -1 : 1;
209-
});
210-
return sortedItems;
211-
}
212-
213185
function matchFilter(
214186
filter: DiscreteFilter | IntervalFilter,
215187
value: number | DiscreteHparamValue | undefined
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/* Copyright 2023 The TensorFlow Authors. All Rights Reserved.
2+
3+
Licensed under the Apache License, Version 2.0 (the "License");
4+
you may not use this file except in compliance with the License.
5+
You may obtain a copy of the License at
6+
7+
http://www.apache.org/licenses/LICENSE-2.0
8+
9+
Unless required by applicable law or agreed to in writing, software
10+
distributed under the License is distributed on an "AS IS" BASIS,
11+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
See the License for the specific language governing permissions and
13+
limitations under the License.
14+
==============================================================================*/
15+
import {
16+
SortingInfo,
17+
SortingOrder,
18+
TableData,
19+
} from '../../../widgets/data_table/types';
20+
import {ExperimentAlias} from '../../../experiments/types';
21+
22+
enum UndefinedStrategy {
23+
BEFORE,
24+
AFTER,
25+
}
26+
27+
interface SortOptions {
28+
insertUndefined: UndefinedStrategy;
29+
}
30+
31+
const POTENTIALLY_NUMERIC_TYPES = new Set(['string', 'number']);
32+
33+
const DEFAULT_SORT_OPTIONS: SortOptions = {
34+
insertUndefined: UndefinedStrategy.AFTER,
35+
};
36+
37+
export function parseNumericPrefix(value: string | number) {
38+
if (typeof value === 'number') {
39+
return isNaN(value) ? undefined : value;
40+
}
41+
42+
if (!isNaN(parseInt(value))) {
43+
return parseInt(value);
44+
}
45+
46+
for (let i = 0; i < value.length; i++) {
47+
if (isNaN(parseInt(value[i]))) {
48+
if (i === 0) return;
49+
return parseInt(value.slice(0, i));
50+
}
51+
}
52+
53+
return;
54+
}
55+
56+
export function sortTableDataItems(
57+
items: TableData[],
58+
sort: SortingInfo
59+
): TableData[] {
60+
const sortedItems = [...items];
61+
62+
sortedItems.sort((a, b) => {
63+
let aValue = a[sort.name];
64+
let bValue = b[sort.name];
65+
66+
if (sort.name === 'experimentAlias') {
67+
aValue = (aValue as ExperimentAlias).aliasNumber;
68+
bValue = (bValue as ExperimentAlias).aliasNumber;
69+
}
70+
71+
if (aValue === bValue) {
72+
return 0;
73+
}
74+
75+
if (aValue === undefined || bValue === undefined) {
76+
return compareValues(aValue, bValue);
77+
}
78+
79+
if (
80+
POTENTIALLY_NUMERIC_TYPES.has(typeof aValue) &&
81+
POTENTIALLY_NUMERIC_TYPES.has(typeof bValue)
82+
) {
83+
const aPrefix = parseNumericPrefix(aValue as string | number);
84+
const bPrefix = parseNumericPrefix(bValue as string | number);
85+
// Show runs with numbers before to runs without numbers
86+
if (
87+
(aPrefix === undefined || bPrefix === undefined) &&
88+
aPrefix !== bPrefix
89+
) {
90+
return compareValues(aPrefix, bPrefix, {
91+
insertUndefined: UndefinedStrategy.BEFORE,
92+
});
93+
}
94+
if (aPrefix !== undefined && bPrefix !== undefined) {
95+
if (aPrefix === bPrefix) {
96+
const aPostfix =
97+
aValue.toString().slice(aPrefix.toString().length) || undefined;
98+
const bPostfix =
99+
bValue.toString().slice(bPrefix.toString().length) || undefined;
100+
return compareValues(aPostfix, bPostfix, {
101+
insertUndefined: UndefinedStrategy.BEFORE,
102+
});
103+
}
104+
105+
return compareValues(aPrefix, bPrefix);
106+
}
107+
}
108+
109+
return compareValues(aValue, bValue);
110+
});
111+
return sortedItems;
112+
113+
function compareValues(
114+
a: TableData[string] | undefined,
115+
b: TableData[string] | undefined,
116+
{insertUndefined}: SortOptions = DEFAULT_SORT_OPTIONS
117+
) {
118+
if (a === b) {
119+
return 0;
120+
}
121+
122+
if (a === undefined) {
123+
return insertUndefined === UndefinedStrategy.AFTER ? 1 : -1;
124+
}
125+
if (b === undefined) {
126+
return insertUndefined === UndefinedStrategy.AFTER ? -1 : 1;
127+
}
128+
129+
return a < b === (sort.order === SortingOrder.ASCENDING) ? -1 : 1;
130+
}
131+
}

0 commit comments

Comments
 (0)