diff --git a/tensorboard/webapp/runs/views/runs_table/BUILD b/tensorboard/webapp/runs/views/runs_table/BUILD index 4d936d8af1d..6ccc536e0b6 100644 --- a/tensorboard/webapp/runs/views/runs_table/BUILD +++ b/tensorboard/webapp/runs/views/runs_table/BUILD @@ -64,6 +64,7 @@ tf_ng_module( "runs_table_component.ts", "runs_table_container.ts", "runs_table_module.ts", + "sorting_utils.ts", ], assets = [ ":regex_edit_dialog_styles", @@ -131,6 +132,7 @@ tf_ts_library( "regex_edit_dialog_test.ts", "runs_data_table_test.ts", "runs_table_test.ts", + "sorting_utils_test.ts", ], deps = [ ":runs_table", diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts index 7487ab8eddc..cdf7567b6d7 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts @@ -70,7 +70,6 @@ import { ColumnHeader, FilterAddedEvent, SortingInfo, - SortingOrder, TableData, } from '../../../widgets/data_table/types'; import { @@ -101,6 +100,7 @@ import { getPotentialHparamColumns, } from '../../../metrics/views/main_view/common_selectors'; import {runsTableFullScreenToggled} from '../../../core/actions'; +import {sortTableDataItems} from './sorting_utils'; const getRunsLoading = createSelector< State, @@ -182,34 +182,6 @@ function sortRunTableItems( return sortedItems; } -function sortTableDataItems( - items: TableData[], - sort: SortingInfo -): TableData[] { - const sortedItems = [...items]; - - sortedItems.sort((a, b) => { - let aValue = a[sort.name]; - let bValue = b[sort.name]; - - if (sort.name === 'experimentAlias') { - aValue = (aValue as ExperimentAlias).aliasNumber; - bValue = (bValue as ExperimentAlias).aliasNumber; - } - - if (aValue === bValue) { - return 0; - } - - if (aValue === undefined || bValue === undefined) { - return bValue === undefined ? -1 : 1; - } - - return aValue < bValue === (sort.order === SortingOrder.ASCENDING) ? -1 : 1; - }); - return sortedItems; -} - function matchFilter( filter: DiscreteFilter | IntervalFilter, value: number | DiscreteHparamValue | undefined diff --git a/tensorboard/webapp/runs/views/runs_table/sorting_utils.ts b/tensorboard/webapp/runs/views/runs_table/sorting_utils.ts new file mode 100644 index 00000000000..cf67fbaf5f7 --- /dev/null +++ b/tensorboard/webapp/runs/views/runs_table/sorting_utils.ts @@ -0,0 +1,131 @@ +/* Copyright 2023 The TensorFlow Authors. All Rights Reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +==============================================================================*/ +import { + SortingInfo, + SortingOrder, + TableData, +} from '../../../widgets/data_table/types'; +import {ExperimentAlias} from '../../../experiments/types'; + +enum UndefinedStrategy { + BEFORE, + AFTER, +} + +interface SortOptions { + insertUndefined: UndefinedStrategy; +} + +const POTENTIALLY_NUMERIC_TYPES = new Set(['string', 'number']); + +const DEFAULT_SORT_OPTIONS: SortOptions = { + insertUndefined: UndefinedStrategy.AFTER, +}; + +export function parseNumericPrefix(value: string | number) { + if (typeof value === 'number') { + return isNaN(value) ? undefined : value; + } + + if (!isNaN(parseInt(value))) { + return parseInt(value); + } + + for (let i = 0; i < value.length; i++) { + if (isNaN(parseInt(value[i]))) { + if (i === 0) return; + return parseInt(value.slice(0, i)); + } + } + + return; +} + +export function sortTableDataItems( + items: TableData[], + sort: SortingInfo +): TableData[] { + const sortedItems = [...items]; + + sortedItems.sort((a, b) => { + let aValue = a[sort.name]; + let bValue = b[sort.name]; + + if (sort.name === 'experimentAlias') { + aValue = (aValue as ExperimentAlias).aliasNumber; + bValue = (bValue as ExperimentAlias).aliasNumber; + } + + if (aValue === bValue) { + return 0; + } + + if (aValue === undefined || bValue === undefined) { + return compareValues(aValue, bValue); + } + + if ( + POTENTIALLY_NUMERIC_TYPES.has(typeof aValue) && + POTENTIALLY_NUMERIC_TYPES.has(typeof bValue) + ) { + const aPrefix = parseNumericPrefix(aValue as string | number); + const bPrefix = parseNumericPrefix(bValue as string | number); + // Show runs with numbers before to runs without numbers + if ( + (aPrefix === undefined || bPrefix === undefined) && + aPrefix !== bPrefix + ) { + return compareValues(aPrefix, bPrefix, { + insertUndefined: UndefinedStrategy.BEFORE, + }); + } + if (aPrefix !== undefined && bPrefix !== undefined) { + if (aPrefix === bPrefix) { + const aPostfix = + aValue.toString().slice(aPrefix.toString().length) || undefined; + const bPostfix = + bValue.toString().slice(bPrefix.toString().length) || undefined; + return compareValues(aPostfix, bPostfix, { + insertUndefined: UndefinedStrategy.BEFORE, + }); + } + + return compareValues(aPrefix, bPrefix); + } + } + + return compareValues(aValue, bValue); + }); + return sortedItems; + + function compareValues( + a: TableData[string] | undefined, + b: TableData[string] | undefined, + {insertUndefined}: SortOptions = DEFAULT_SORT_OPTIONS + ) { + if (a === b) { + return 0; + } + + if (a === undefined) { + return insertUndefined === UndefinedStrategy.AFTER ? 1 : -1; + } + if (b === undefined) { + return insertUndefined === UndefinedStrategy.AFTER ? -1 : 1; + } + + return a < b === (sort.order === SortingOrder.ASCENDING) ? -1 : 1; + } +} diff --git a/tensorboard/webapp/runs/views/runs_table/sorting_utils_test.ts b/tensorboard/webapp/runs/views/runs_table/sorting_utils_test.ts new file mode 100644 index 00000000000..25348ec9396 --- /dev/null +++ b/tensorboard/webapp/runs/views/runs_table/sorting_utils_test.ts @@ -0,0 +1,286 @@ +/* Copyright 2023 The TensorFlow Authors. All Rights Reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +==============================================================================*/ +import {SortingOrder} from '../../../widgets/data_table/types'; +import {parseNumericPrefix, sortTableDataItems} from './sorting_utils'; + +describe('sorting utils', () => { + describe('parseNumericPrefix', () => { + it('returns undefined when a non numeric value is provided', () => { + expect(parseNumericPrefix('')).toBeUndefined(); + expect(parseNumericPrefix('foo')).toBeUndefined(); + expect(parseNumericPrefix('foo123')).toBeUndefined(); + expect(parseNumericPrefix(NaN)).toBeUndefined(); + }); + + it('returns all leading numbers from a string', () => { + expect(parseNumericPrefix('0')).toEqual(0); + expect(parseNumericPrefix('123')).toEqual(123); + expect(parseNumericPrefix('123train')).toEqual(123); + expect(parseNumericPrefix('123/')).toEqual(123); + expect(parseNumericPrefix('123/foo')).toEqual(123); + expect(parseNumericPrefix('123/foo/456')).toEqual(123); + }); + + it('returns numbers when provided', () => { + expect(parseNumericPrefix(123)).toEqual(123); + }); + }); + + describe('sortTableDataItems', () => { + it('sorts experimentAlias by alias number', () => { + expect( + sortTableDataItems( + [ + { + id: 'row 1 id', + experimentAlias: { + aliasNumber: 5, + }, + }, + { + id: 'row 2 id', + experimentAlias: { + aliasNumber: 3, + }, + }, + ], + { + order: SortingOrder.ASCENDING, + name: 'experimentAlias', + } + ) + ).toEqual([ + { + id: 'row 2 id', + experimentAlias: { + aliasNumber: 3, + }, + }, + { + id: 'row 1 id', + experimentAlias: { + aliasNumber: 5, + }, + }, + ]); + }); + + it('sorts runs by their leading numbers', () => { + expect( + sortTableDataItems( + [ + { + id: 'row 1 id', + name: '1/myrun', + }, + { + id: 'row 2 id', + name: '2/myrun', + }, + { + id: 'row 3 id', + name: '10/myrun', + }, + ], + { + order: SortingOrder.ASCENDING, + name: 'name', + } + ) + ).toEqual([ + { + id: 'row 1 id', + name: '1/myrun', + }, + { + id: 'row 2 id', + name: '2/myrun', + }, + { + id: 'row 3 id', + name: '10/myrun', + }, + ]); + }); + + it('sorts runs with purely numeric run names before runs with leading numbers', () => { + expect( + sortTableDataItems( + [ + { + id: 'row 1 id', + name: '0', + }, + { + id: 'row 2 id', + name: '0/myrun2', + }, + { + id: 'row 3 id', + name: '0/myrun1', + }, + ], + { + order: SortingOrder.ASCENDING, + name: 'name', + } + ) + ).toEqual([ + { + id: 'row 1 id', + name: '0', + }, + { + id: 'row 3 id', + name: '0/myrun1', + }, + { + id: 'row 2 id', + name: '0/myrun2', + }, + ]); + }); + + it('sorts runs with string names', () => { + expect( + sortTableDataItems( + [ + { + id: 'row 1 id', + name: 'aaa', + }, + { + id: 'row 2 id', + name: 'bbb', + }, + { + id: 'row 3 id', + name: 'ccc', + }, + ], + { + order: SortingOrder.ASCENDING, + name: 'name', + } + ) + ).toEqual([ + { + id: 'row 1 id', + name: 'aaa', + }, + { + id: 'row 2 id', + name: 'bbb', + }, + { + id: 'row 3 id', + name: 'ccc', + }, + ]); + }); + + it('shows runs without numbers before runs with numbers', () => { + expect( + sortTableDataItems( + [ + { + id: 'row 1 id', + name: 'aaa', + }, + { + id: 'row 2 id', + name: '1aaa', + }, + { + id: 'row 3 id', + name: '2bbb', + }, + ], + { + order: SortingOrder.ASCENDING, + name: 'name', + } + ) + ).toEqual([ + { + id: 'row 1 id', + name: 'aaa', + }, + { + id: 'row 2 id', + name: '1aaa', + }, + { + id: 'row 3 id', + name: '2bbb', + }, + ]); + }); + + it('places undefined values at the end', () => { + const input: any = [ + { + id: 'row 1 id', + foo: '1/myrun', + }, + { + id: 'row 2 id', + }, + { + id: 'row 3 id', + foo: '10/myrun', + }, + ]; + + expect( + sortTableDataItems(input, { + order: SortingOrder.ASCENDING, + name: 'foo', + }) + ).toEqual([ + { + id: 'row 1 id', + foo: '1/myrun', + }, + { + id: 'row 3 id', + foo: '10/myrun', + }, + { + id: 'row 2 id', + }, + ]); + + expect( + sortTableDataItems(input, { + order: SortingOrder.DESCENDING, + name: 'foo', + }) + ).toEqual([ + { + id: 'row 3 id', + foo: '10/myrun', + }, + { + id: 'row 1 id', + foo: '1/myrun', + }, + { + id: 'row 2 id', + }, + ]); + }); + }); +});