From 0714a8d2ef16bceb81e278ab62a5754224b51854 Mon Sep 17 00:00:00 2001 From: nabenabe0928 Date: Thu, 28 Dec 2023 06:01:19 +0100 Subject: [PATCH 1/4] Refactor value comparator --- optuna_dashboard/ts/components/TrialTable.tsx | 93 ++++++++----------- 1 file changed, 37 insertions(+), 56 deletions(-) diff --git a/optuna_dashboard/ts/components/TrialTable.tsx b/optuna_dashboard/ts/components/TrialTable.tsx index efc79626..c2e4a650 100644 --- a/optuna_dashboard/ts/components/TrialTable.tsx +++ b/optuna_dashboard/ts/components/TrialTable.tsx @@ -23,29 +23,37 @@ export const TrialTable: FC<{ toCellValue: (i) => trials[i].state.toString(), }, ] + const valueComparator = ( + firstVal?: TrialValueNumber, + secondVal?: TrialValueNumber, + ascending: boolean = true + ): number => { + if (firstVal === secondVal) { + return 0 + } + if (firstVal === undefined) { + return ascending ? -1 : 1 + } else if (secondVal === undefined) { + return ascending ? 1 : -1 + } + if (firstVal === "-inf" || secondVal === "inf") { + return 1 + } else if (secondVal === "-inf" || firstVal === "inf") { + return -1 + } + return firstVal < secondVal ? 1 : -1 + } if (studyDetail === null || studyDetail.directions.length === 1) { columns.push({ field: "values", label: "Value", sortable: true, less: (firstEl, secondEl, ascending): number => { - const firstVal = firstEl.values?.[0] - const secondVal = secondEl.values?.[0] - - if (firstVal === secondVal) { - return 0 - } - if (firstVal === undefined) { - return ascending ? -1 : 1 - } else if (secondVal === undefined) { - return ascending ? 1 : -1 - } - if (firstVal === "-inf" || secondVal === "inf") { - return 1 - } else if (secondVal === "-inf" || firstVal === "inf") { - return -1 - } - return firstVal < secondVal ? 1 : -1 + return valueComparator( + firstEl.values?.[0], + secondEl.values?.[0], + ascending + ) }, toCellValue: (i) => { if (trials[i].values === undefined) { @@ -64,23 +72,11 @@ export const TrialTable: FC<{ : `Objective ${objectiveId}`, sortable: true, less: (firstEl, secondEl, ascending): number => { - const firstVal = firstEl.values?.[objectiveId] - const secondVal = secondEl.values?.[objectiveId] - - if (firstVal === secondVal) { - return 0 - } - if (firstVal === undefined) { - return ascending ? -1 : 1 - } else if (secondVal === undefined) { - return ascending ? 1 : -1 - } - if (firstVal === "-inf" || secondVal === "inf") { - return 1 - } else if (secondVal === "-inf" || firstVal === "inf") { - return -1 - } - return firstVal < secondVal ? 1 : -1 + return valueComparator( + firstEl.values?.[objectiveId], + secondEl.values?.[objectiveId], + ascending + ) }, toCellValue: (i) => { if (trials[i].values === undefined) { @@ -117,16 +113,7 @@ export const TrialTable: FC<{ const secondVal = secondEl.params.find( (p) => p.name === s.name )?.param_internal_value - - if (firstVal === secondVal) { - return 0 - } else if (firstVal && secondVal) { - return firstVal < secondVal ? 1 : -1 - } else if (firstVal) { - return -1 - } else { - return 1 - } + return valueComparator(firstVal, secondVal) }, }) }) @@ -151,22 +138,16 @@ export const TrialTable: FC<{ sortable: attr_spec.sortable, // eslint-disable-next-line @typescript-eslint/no-unused-vars less: (firstEl, secondEl, _): number => { - const firstVal = firstEl.user_attrs.find( + const firstValString = firstEl.user_attrs.find( (attr) => attr.key === attr_spec.key )?.value - const secondVal = secondEl.user_attrs.find( + const secondValString = secondEl.user_attrs.find( (attr) => attr.key === attr_spec.key )?.value - - if (firstVal === secondVal) { - return 0 - } else if (firstVal && secondVal) { - return Number(firstVal) < Number(secondVal) ? 1 : -1 - } else if (firstVal) { - return -1 - } else { - return 1 - } + return valueComparator( + Number(firstValString) ?? firstValString, + Number(secondValString) ?? secondValString + ) }, }) }) From 8e86bc6c0e51ae81b03271b9b1978e860a1dc841 Mon Sep 17 00:00:00 2001 From: nabenabe0928 Date: Thu, 28 Dec 2023 06:58:46 +0100 Subject: [PATCH 2/4] Adapt to categorical dynamic without null --- optuna_dashboard/ts/components/DataGrid.tsx | 2 +- optuna_dashboard/ts/components/TrialTable.tsx | 58 +++++++------------ 2 files changed, 23 insertions(+), 37 deletions(-) diff --git a/optuna_dashboard/ts/components/DataGrid.tsx b/optuna_dashboard/ts/components/DataGrid.tsx index adf64e77..9763d103 100644 --- a/optuna_dashboard/ts/components/DataGrid.tsx +++ b/optuna_dashboard/ts/components/DataGrid.tsx @@ -33,7 +33,7 @@ interface DataGridColumn { label: string sortable?: boolean less?: (a: T, b: T, ascending: boolean) => number - filterChoices?: string[] + filterChoices?: (string | null)[] toCellValue?: (rowIndex: number) => string | React.ReactNode padding?: "normal" | "checkbox" | "none" } diff --git a/optuna_dashboard/ts/components/TrialTable.tsx b/optuna_dashboard/ts/components/TrialTable.tsx index c2e4a650..daf39439 100644 --- a/optuna_dashboard/ts/components/TrialTable.tsx +++ b/optuna_dashboard/ts/components/TrialTable.tsx @@ -87,46 +87,32 @@ export const TrialTable: FC<{ })) columns.push(...objectiveColumns) } - if ( - studyDetail?.union_search_space.length === - studyDetail?.intersection_search_space.length - ) { - studyDetail?.intersection_search_space.forEach((s) => { - const sortable = s.distribution.type !== "CategoricalDistribution" - const filterChoices = - s.distribution.type === "CategoricalDistribution" - ? s.distribution.choices.map((c) => c.value) - : undefined - columns.push({ - field: "params", - label: `Param ${s.name}`, - toCellValue: (i) => - trials[i].params.find((p) => p.name === s.name) - ?.param_external_value || null, - sortable: sortable, - filterChoices: filterChoices, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - less: (firstEl, secondEl, _): number => { - const firstVal = firstEl.params.find( - (p) => p.name === s.name - )?.param_internal_value - const secondVal = secondEl.params.find( - (p) => p.name === s.name - )?.param_internal_value - return valueComparator(firstVal, secondVal) - }, - }) - }) - } else { + studyDetail?.union_search_space.forEach((s) => { + const sortable = s.distribution.type !== "CategoricalDistribution" + const filterChoices = + s.distribution.type === "CategoricalDistribution" + ? s.distribution.choices.map((c) => c.value) + : undefined columns.push({ field: "params", - label: "Params", + label: `Param ${s.name}`, toCellValue: (i) => - trials[i].params - .map((p) => p.name + ": " + p.param_external_value) - .join(", "), + trials[i].params.find((p) => p.name === s.name) + ?.param_external_value || null, + sortable: sortable, + filterChoices: filterChoices, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + less: (firstEl, secondEl, _): number => { + const firstVal = firstEl.params.find( + (p) => p.name === s.name + )?.param_internal_value + const secondVal = secondEl.params.find( + (p) => p.name === s.name + )?.param_internal_value + return valueComparator(firstVal, secondVal) + }, }) - } + }) studyDetail?.union_user_attrs.forEach((attr_spec) => { columns.push({ From 797a8cdce5c1541e25f8ec96a913432444fdf123 Mon Sep 17 00:00:00 2001 From: nabenabe0928 Date: Thu, 28 Dec 2023 07:37:39 +0100 Subject: [PATCH 3/4] Add missing value filter --- optuna_dashboard/ts/components/DataGrid.tsx | 2 +- optuna_dashboard/ts/components/TrialTable.tsx | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/optuna_dashboard/ts/components/DataGrid.tsx b/optuna_dashboard/ts/components/DataGrid.tsx index 9763d103..93c95eb2 100644 --- a/optuna_dashboard/ts/components/DataGrid.tsx +++ b/optuna_dashboard/ts/components/DataGrid.tsx @@ -282,7 +282,7 @@ function DataGridHeaderColumn(props: { )} - {choice} + {choice ?? "(missing value)"} ))} diff --git a/optuna_dashboard/ts/components/TrialTable.tsx b/optuna_dashboard/ts/components/TrialTable.tsx index daf39439..bd26921b 100644 --- a/optuna_dashboard/ts/components/TrialTable.tsx +++ b/optuna_dashboard/ts/components/TrialTable.tsx @@ -87,20 +87,26 @@ export const TrialTable: FC<{ })) columns.push(...objectiveColumns) } + const isDynamicSpace = + studyDetail?.union_search_space.length !== + studyDetail?.intersection_search_space.length studyDetail?.union_search_space.forEach((s) => { const sortable = s.distribution.type !== "CategoricalDistribution" const filterChoices = s.distribution.type === "CategoricalDistribution" ? s.distribution.choices.map((c) => c.value) : undefined + const filterChoicesWithNull: (string | null)[] | undefined = filterChoices + ? [...filterChoices, null] + : undefined columns.push({ field: "params", label: `Param ${s.name}`, toCellValue: (i) => - trials[i].params.find((p) => p.name === s.name) - ?.param_external_value || null, + trials[i].params.find((p) => p.name === s.name)?.param_external_value || + null, sortable: sortable, - filterChoices: filterChoices, + filterChoices: isDynamicSpace ? filterChoicesWithNull : filterChoices, // eslint-disable-next-line @typescript-eslint/no-unused-vars less: (firstEl, secondEl, _): number => { const firstVal = firstEl.params.find( From c13654f19bcb3b248d18bb58b65b0f6cfa5e1ae5 Mon Sep 17 00:00:00 2001 From: nabenabe0928 Date: Tue, 13 Feb 2024 17:17:41 +0100 Subject: [PATCH 4/4] Apply the change suggested by umezawa --- optuna_dashboard/ts/components/TrialTable.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/optuna_dashboard/ts/components/TrialTable.tsx b/optuna_dashboard/ts/components/TrialTable.tsx index bd26921b..667fc934 100644 --- a/optuna_dashboard/ts/components/TrialTable.tsx +++ b/optuna_dashboard/ts/components/TrialTable.tsx @@ -92,13 +92,16 @@ export const TrialTable: FC<{ studyDetail?.intersection_search_space.length studyDetail?.union_search_space.forEach((s) => { const sortable = s.distribution.type !== "CategoricalDistribution" - const filterChoices = + const filterChoices: (string | null)[] | undefined = s.distribution.type === "CategoricalDistribution" ? s.distribution.choices.map((c) => c.value) : undefined - const filterChoicesWithNull: (string | null)[] | undefined = filterChoices - ? [...filterChoices, null] - : undefined + const hasMissingValue = trials.some( + (t) => !t.params.some((p) => p.name === s.name) + ) + if (filterChoices !== undefined && isDynamicSpace && hasMissingValue) { + filterChoices.push(null) + } columns.push({ field: "params", label: `Param ${s.name}`, @@ -106,7 +109,7 @@ export const TrialTable: FC<{ trials[i].params.find((p) => p.name === s.name)?.param_external_value || null, sortable: sortable, - filterChoices: isDynamicSpace ? filterChoicesWithNull : filterChoices, + filterChoices: filterChoices, // eslint-disable-next-line @typescript-eslint/no-unused-vars less: (firstEl, secondEl, _): number => { const firstVal = firstEl.params.find(