From e39e59306b75fbf37b5a28d3566821e38912128e Mon Sep 17 00:00:00 2001 From: c-bata Date: Tue, 9 May 2023 20:31:24 +0900 Subject: [PATCH 01/10] Add an optional option for TextInputWidget --- optuna_dashboard/_form_widget.py | 9 +++++++- optuna_dashboard/ts/action.ts | 4 ++-- optuna_dashboard/ts/apiClient.ts | 2 +- .../ts/components/ObjectiveForm.tsx | 22 ++++++++++++------- optuna_dashboard/ts/types/index.d.ts | 1 + 5 files changed, 26 insertions(+), 12 deletions(-) diff --git a/optuna_dashboard/_form_widget.py b/optuna_dashboard/_form_widget.py index 8afda925..045c3512 100644 --- a/optuna_dashboard/_form_widget.py +++ b/optuna_dashboard/_form_widget.py @@ -46,7 +46,7 @@ if TYPE_CHECKING: ) TextInputWidgetJSON = TypedDict( "TextInputWidgetJSON", - {"type": Literal["text"], "description": Optional[str], "user_attr_key": Optional[str]}, + {"type": Literal["text"], "description": Optional[str], "user_attr_key": Optional[str], "optional": bool}, ) UserAttrRefJSON = TypedDict("UserAttrRefJSON", {"type": Literal["user_attr"], "key": str}) FormWidgetJSON = TypedDict( @@ -130,12 +130,14 @@ class SliderWidget: class TextInputWidget: description: Optional[str] = None user_attr_key: Optional[str] = None + optional: bool = False def to_dict(self) -> TextInputWidgetJSON: return { "type": "text", "description": self.description, "user_attr_key": self.user_attr_key, + "optional": self.optional, } @classmethod @@ -144,6 +146,7 @@ class TextInputWidget: return cls( description=d.get("description"), user_attr_key=d.get("user_attr_key"), + optional=d.get("optional", False), ) @@ -212,6 +215,10 @@ def register_objective_form_widgets( not isinstance(w, ObjectiveUserAttrRef) and w.user_attr_key is not None for w in widgets ): warnings.warn("`user_attr_key` specified, but it will not be used.") + if any( + isinstance(w, TextInputWidget) and w.optional is False for w in widgets + ): + raise ValueError("TextInputWidget.optional must be False.") form_widgets: FormWidgetJSON = { "output_type": "objective", "widgets": [w.to_dict() for w in widgets], diff --git a/optuna_dashboard/ts/action.ts b/optuna_dashboard/ts/action.ts index eaeecab8..defef21d 100644 --- a/optuna_dashboard/ts/action.ts +++ b/optuna_dashboard/ts/action.ts @@ -155,7 +155,7 @@ export const actionCreator = () => { const setTrialUserAttrs = ( studyId: number, index: number, - user_attrs: { [key: string]: number } + user_attrs: { [key: string]: number | string } ) => { const newTrial: Trial = Object.assign( {}, @@ -529,7 +529,7 @@ export const actionCreator = () => { const saveTrialUserAttrs = ( studyId: number, trialId: number, - user_attrs: { [key: string]: number } + user_attrs: { [key: string]: string | number } ): void => { console.log("user_attrs", user_attrs) const message = `id=${trialId}, user_attrs=${JSON.stringify(user_attrs)}` diff --git a/optuna_dashboard/ts/apiClient.ts b/optuna_dashboard/ts/apiClient.ts index 578bcc03..3e291310 100644 --- a/optuna_dashboard/ts/apiClient.ts +++ b/optuna_dashboard/ts/apiClient.ts @@ -283,7 +283,7 @@ export const tellTrialAPI = ( export const saveTrialUserAttrsAPI = ( trialId: number, - user_attrs: { [key: string]: number } + user_attrs: { [key: string]: number | string } ): Promise => { const req = { user_attrs: user_attrs } diff --git a/optuna_dashboard/ts/components/ObjectiveForm.tsx b/optuna_dashboard/ts/components/ObjectiveForm.tsx index 09a44776..8c564668 100644 --- a/optuna_dashboard/ts/components/ObjectiveForm.tsx +++ b/optuna_dashboard/ts/components/ObjectiveForm.tsx @@ -24,12 +24,12 @@ export const ObjectiveForm: FC<{ }> = ({ trial, directions, names, formWidgets }) => { const theme = useTheme() const action = actionCreator() - const [values, setValues] = useState<(number | null)[]>( + const [values, setValues] = useState<(number | string | null)[]>( formWidgets.widgets.map((widget) => { if (widget === undefined) { return null } else if (widget.type === "text") { - return null + return "" } else if (widget.type === "choice") { return widget.values.at(0) || null } else if (widget.type === "slider") { @@ -58,8 +58,14 @@ export const ObjectiveForm: FC<{ } const disableSubmit = useMemo( - () => values.findIndex((v) => v === null) >= 0, - [values] + () => values.findIndex((v, i) => { + const w = formWidgets.widgets[i] + if (formWidgets.output_type === "user_attr" && w.type === "text" && w.optional) { + return false + } + return v === null + }) >= 0, + [values, formWidgets] ) const handleSubmit = (e: React.MouseEvent): void => { @@ -74,7 +80,7 @@ export const ObjectiveForm: FC<{ const user_attrs = Object.fromEntries( formWidgets.widgets.map((widget, i) => [ widget.user_attr_key, - values[i], + values[i] !== null ? values[i] : "", ]) ) action.saveTrialUserAttrs(trial.study_id, trial.trial_id, user_attrs) @@ -137,15 +143,15 @@ export const ObjectiveForm: FC<{ }} delay={500} textFieldProps={{ - required: true, + required: !widget.optional, autoFocus: true, fullWidth: true, helperText: - value === null || value === undefined + !widget.optional && (value === null || value === undefined) ? `Please input the float number.` : "", type: "text", - inputProps: { + inputProps: formWidgets.output_type === "user_attr" ? undefined : { pattern: "[-+]?[0-9]*.?[0-9]+([eE][-+]?[0-9]+)?", }, }} diff --git a/optuna_dashboard/ts/types/index.d.ts b/optuna_dashboard/ts/types/index.d.ts index 24548a2c..b58786d7 100644 --- a/optuna_dashboard/ts/types/index.d.ts +++ b/optuna_dashboard/ts/types/index.d.ts @@ -151,6 +151,7 @@ type ObjectiveSliderWidget = { type ObjectiveTextInputWidget = { type: "text" description: string + optional: boolean user_attr_key?: string } From c9223d753cc21a7dfaa797af6b3189309d525248 Mon Sep 17 00:00:00 2001 From: c-bata Date: Tue, 9 May 2023 22:15:19 +0900 Subject: [PATCH 02/10] Split TextInputWidget --- optuna_dashboard/_serializer.py | 2 + optuna_dashboard/ts/action.ts | 1 - .../ts/components/ObjectiveForm.tsx | 163 ++++++++++++------ 3 files changed, 115 insertions(+), 51 deletions(-) diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index e789e18a..64ff7cf3 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -91,6 +91,8 @@ def serialize_attrs(attrs: dict[str, Any]) -> list[Attribute]: value: str if isinstance(v, bytes): value = "" + elif isinstance(v, str): + value = v else: value = json.dumps(v) value = value[:MAX_ATTR_LENGTH] if len(value) > MAX_ATTR_LENGTH else value diff --git a/optuna_dashboard/ts/action.ts b/optuna_dashboard/ts/action.ts index defef21d..a97cd522 100644 --- a/optuna_dashboard/ts/action.ts +++ b/optuna_dashboard/ts/action.ts @@ -531,7 +531,6 @@ export const actionCreator = () => { trialId: number, user_attrs: { [key: string]: string | number } ): void => { - console.log("user_attrs", user_attrs) const message = `id=${trialId}, user_attrs=${JSON.stringify(user_attrs)}` saveTrialUserAttrsAPI(trialId, user_attrs) .then(() => { diff --git a/optuna_dashboard/ts/components/ObjectiveForm.tsx b/optuna_dashboard/ts/components/ObjectiveForm.tsx index 8c564668..df2617af 100644 --- a/optuna_dashboard/ts/components/ObjectiveForm.tsx +++ b/optuna_dashboard/ts/components/ObjectiveForm.tsx @@ -24,31 +24,35 @@ export const ObjectiveForm: FC<{ }> = ({ trial, directions, names, formWidgets }) => { const theme = useTheme() const action = actionCreator() - const [values, setValues] = useState<(number | string | null)[]>( + const [values, setValues] = useState<(number | string)[]>( formWidgets.widgets.map((widget) => { - if (widget === undefined) { - return null - } else if (widget.type === "text") { + if (widget.type === "text") { return "" } else if (widget.type === "choice") { - return widget.values.at(0) || null + const value = widget.values.at(0) + if (value === undefined) { + console.error("Must not reach ehere") + return 0 + } + return value } else if (widget.type === "slider") { return widget.min } else if (widget.type === "user_attr") { const attr = trial.user_attrs.find((attr) => attr.key == widget.key) if (attr === undefined) { - return null + return 0 } else { const n = Number(attr.value) - return isNaN(n) ? null : n + return isNaN(n) ? 0 : n } } else { - return null + console.error("Must not reach here") + return "" } }) ) - const setValue = (objectiveId: number, value: number | null) => { + const setValue = (objectiveId: number, value: number | string) => { const newValues = [...values] if (newValues.length <= objectiveId) { return @@ -58,13 +62,18 @@ export const ObjectiveForm: FC<{ } const disableSubmit = useMemo( - () => values.findIndex((v, i) => { + () => + values.findIndex((v, i) => { const w = formWidgets.widgets[i] - if (formWidgets.output_type === "user_attr" && w.type === "text" && w.optional) { - return false + if ( + formWidgets.output_type === "user_attr" && + w.type === "text" && + w.optional + ) { + return false } return v === null - }) >= 0, + }) >= 0, [values, formWidgets] ) @@ -104,13 +113,20 @@ export const ObjectiveForm: FC<{ return "Unkown metric name" } + const headerText = + formWidgets.output_type === "user_attr" + ? "Set User Attributes Form" + : directions.length > 1 + ? "Set Objective Values Form" + : "Set Objective Value Form" + return ( <> - {directions.length > 1 ? "Set Objective Values" : "Set Objective Value"} + {headerText} {formWidgets.widgets.map((widget, i) => { - const value = values.at(i) + const value = values.at(i) || "" const key = `objective-${i}` if (widget.type === "text") { return ( - - - {getMetricName(i)} - {widget.description} - - { - const n = Number(s) - if (s.length > 0 && valid && !isNaN(n)) { - setValue(i, n) - return - } else if (values.at(i) !== null) { - setValue(i, null) - } - }} - delay={500} - textFieldProps={{ - required: !widget.optional, - autoFocus: true, - fullWidth: true, - helperText: - !widget.optional && (value === null || value === undefined) - ? `Please input the float number.` - : "", - type: "text", - inputProps: formWidgets.output_type === "user_attr" ? undefined : { - pattern: "[-+]?[0-9]*.?[0-9]+([eE][-+]?[0-9]+)?", - }, - }} - /> - + { + setValue(i, value) + }} + value={value} + /> ) } else if (widget.type === "choice") { return ( @@ -173,11 +169,11 @@ export const ObjectiveForm: FC<{ checked={value === widget.values.at(j)} onChange={(e) => { const selected = widget.values.at(j) + if (selected === undefined) { + console.error("Must not reach here.") + } if (e.target.checked) { - setValue( - i, - selected === undefined ? null : selected - ) + setValue(i, selected || 0) } }} /> @@ -266,6 +262,56 @@ export const ObjectiveForm: FC<{ ) } +const TextInputWidget: FC<{ + widget: ObjectiveTextInputWidget + widgetType: "user_attr" | "objective" + metricName: string + value: number | string + setValue: (value: number | string) => void +}> = ({ widget, widgetType, metricName, value, setValue }) => { + const theme = useTheme() + const inputProps = + widgetType === "objective" + ? { + pattern: "[-+]?[0-9]*.?[0-9]+([eE][-+]?[0-9]+)?", + } + : undefined + const helperText = + !widget.optional && value === "" ? `Please input the float number.` : "" + + return ( + + + {metricName} - {widget.description} + + { + if (widgetType === "user_attr") { + setValue(s) + return + } + + const n = Number(s) + if (s.length > 0 && valid && !isNaN(n)) { + setValue(n) + } else if (value === "") { + setValue("") + } + }} + delay={500} + textFieldProps={{ + type: "text", + autoFocus: true, + fullWidth: true, + required: !widget.optional, + helperText, + inputProps, + }} + /> + + ) +} + export const ReadonlyObjectiveForm: FC<{ trial: Trial directions: StudyDirection[] @@ -289,6 +335,23 @@ export const ReadonlyObjectiveForm: FC<{ } return "Unkown metric name" } + + const getValue = (i: number): string | TrialValueNumber => { + if (formWidgets.output_type === "user_attr") { + const widget = formWidgets.widgets[i] as UserAttrFormWidget + return ( + trial.user_attrs.find((attr) => attr.key === widget.user_attr_key) + ?.value || "" + ) + } + const value = trial.values?.at(i) + if (value === undefined) { + console.error("Must not reach here.") + return 0 + } + return value + } + return ( <> ) From 63ac794d387808fd4eb99edd57ce4f37a69f6fa1 Mon Sep 17 00:00:00 2001 From: c-bata Date: Wed, 10 May 2023 00:28:09 +0900 Subject: [PATCH 03/10] Fix bugs and add refactor changes --- optuna_dashboard/_form_widget.py | 2 +- .../ts/components/ObjectiveForm.tsx | 376 ++++++++++-------- 2 files changed, 202 insertions(+), 176 deletions(-) diff --git a/optuna_dashboard/_form_widget.py b/optuna_dashboard/_form_widget.py index 045c3512..9517251a 100644 --- a/optuna_dashboard/_form_widget.py +++ b/optuna_dashboard/_form_widget.py @@ -216,7 +216,7 @@ def register_objective_form_widgets( ): warnings.warn("`user_attr_key` specified, but it will not be used.") if any( - isinstance(w, TextInputWidget) and w.optional is False for w in widgets + isinstance(w, TextInputWidget) and w.optional for w in widgets ): raise ValueError("TextInputWidget.optional must be False.") form_widgets: FormWidgetJSON = { diff --git a/optuna_dashboard/ts/components/ObjectiveForm.tsx b/optuna_dashboard/ts/components/ObjectiveForm.tsx index df2617af..74a08f18 100644 --- a/optuna_dashboard/ts/components/ObjectiveForm.tsx +++ b/optuna_dashboard/ts/components/ObjectiveForm.tsx @@ -1,4 +1,4 @@ -import React, { FC, useMemo, useState } from "react" +import React, { FC, ReactNode, useMemo, useState } from "react" import { Typography, Box, @@ -16,6 +16,12 @@ import { import { DebouncedInputTextField } from "./Debounce" import { actionCreator } from "../action" +type WidgetState = { + isValid: boolean + value: number | string + render: () => ReactNode +} + export const ObjectiveForm: FC<{ trial: Trial directions: StudyDirection[] @@ -24,61 +30,63 @@ export const ObjectiveForm: FC<{ }> = ({ trial, directions, names, formWidgets }) => { const theme = useTheme() const action = actionCreator() - const [values, setValues] = useState<(number | string)[]>( - formWidgets.widgets.map((widget) => { - if (widget.type === "text") { - return "" - } else if (widget.type === "choice") { - const value = widget.values.at(0) - if (value === undefined) { - console.error("Must not reach ehere") - return 0 - } - return value - } else if (widget.type === "slider") { - return widget.min - } else if (widget.type === "user_attr") { - const attr = trial.user_attrs.find((attr) => attr.key == widget.key) - if (attr === undefined) { - return 0 - } else { - const n = Number(attr.value) - return isNaN(n) ? 0 : n - } - } else { - console.error("Must not reach here") - return "" - } - }) - ) - const setValue = (objectiveId: number, value: number | string) => { - const newValues = [...values] - if (newValues.length <= objectiveId) { - return + const getMetricName = (i: number): string => { + if (formWidgets.output_type == "objective") { + if (names.at(i) !== undefined) { + return names[i] + } + return directions.length == 1 ? "Objective" : `Objective ${i}` + } else if (formWidgets.output_type == "user_attr") { + const key = formWidgets.widgets.at(i)?.user_attr_key + if (key !== undefined) { + return key + } } - newValues[objectiveId] = value - setValues(newValues) + console.error("Must not reach here") + return "Unknown" } + const widgetStates = formWidgets.widgets + .map((w, i) => { + const key = `${formWidgets.output_type}-${i}` + if (w.type === "text") { + return useTextInputWidget( + key, + formWidgets.output_type, + w, + getMetricName(i) + ) + } else if (w.type === "choice") { + return useChoiceWidget( + key, + formWidgets.output_type, + w, + getMetricName(i) + ) + } else if (w.type === "slider") { + return useSliderWidget( + key, + formWidgets.output_type, + w, + getMetricName(i) + ) + } else if (w.type === "user_attr") { + return useUserAttrRefWidget(key, w, getMetricName(i), trial) + } + console.error("Must not reach here") + return undefined + }) + .filter((w): w is WidgetState => w !== undefined) + const disableSubmit = useMemo( - () => - values.findIndex((v, i) => { - const w = formWidgets.widgets[i] - if ( - formWidgets.output_type === "user_attr" && - w.type === "text" && - w.optional - ) { - return false - } - return v === null - }) >= 0, - [values, formWidgets] + () => !widgetStates.every((ws) => ws.isValid), + [widgetStates] ) const handleSubmit = (e: React.MouseEvent): void => { e.preventDefault() + const values = widgetStates.map((ws) => ws.value) if (formWidgets.output_type == "objective") { const filtered = values.filter((v): v is number => v !== null) if (filtered.length !== directions.length) { @@ -96,23 +104,6 @@ export const ObjectiveForm: FC<{ } } - const getMetricName = (i: number): string => { - if (formWidgets.output_type == "objective") { - const n = names.at(i) - if (n !== undefined) { - return n - } - if (directions.length == 1) { - return `Objective` - } else { - return `Objective ${i}` - } - } else if (formWidgets.output_type == "user_attr") { - return formWidgets.widgets[i].user_attr_key as string - } - return "Unkown metric name" - } - const headerText = formWidgets.output_type === "user_attr" ? "Set User Attributes Form" @@ -138,97 +129,7 @@ export const ObjectiveForm: FC<{ p: theme.spacing(1), }} > - {formWidgets.widgets.map((widget, i) => { - const value = values.at(i) || "" - const key = `objective-${i}` - if (widget.type === "text") { - return ( - { - setValue(i, value) - }} - value={value} - /> - ) - } else if (widget.type === "choice") { - return ( - - - {getMetricName(i)} - {widget.description} - - - {widget.choices.map((c, j) => ( - { - const selected = widget.values.at(j) - if (selected === undefined) { - console.error("Must not reach here.") - } - if (e.target.checked) { - setValue(i, selected || 0) - } - }} - /> - } - label={c} - /> - ))} - - - ) - } else if (widget.type === "slider") { - return ( - - - {getMetricName(i)} - {widget.description} - - - { - // @ts-ignore - setValue(i, e.target.value as number) - }} - defaultValue={widget.min} - min={widget.min} - max={widget.max} - step={widget.step} - marks={ - widget.labels === null || widget.labels.length == 0 - ? true - : widget.labels - } - valueLabelDisplay="auto" - /> - - - ) - } else if (widget.type === "user_attr") { - return ( - - {getMetricName(i)} - - - ) - } - return null - })} + {widgetStates.map((ws) => ws.render())} void -}> = ({ widget, widgetType, metricName, value, setValue }) => { +): WidgetState => { const theme = useTheme() + const [value, setValue] = useState("") + const isValid = useMemo( + () => + widgetType === "user_attr" + ? value !== "" || widget.optional + : value !== "" && !isNaN(Number(value)), + [widget, value] + ) + const inputProps = widgetType === "objective" ? { @@ -278,9 +187,8 @@ const TextInputWidget: FC<{ : undefined const helperText = !widget.optional && value === "" ? `Please input the float number.` : "" - - return ( - + const render = () => ( + {metricName} - {widget.description} @@ -294,7 +202,7 @@ const TextInputWidget: FC<{ const n = Number(s) if (s.length > 0 && valid && !isNaN(n)) { setValue(n) - } else if (value === "") { + } else { setValue("") } }} @@ -310,6 +218,124 @@ const TextInputWidget: FC<{ /> ) + return { isValid, value, render } +} + +export const useChoiceWidget = ( + key: string, + widgetType: "user_attr" | "objective", + widget: ObjectiveChoiceWidget, + metricName: string +): WidgetState => { + const theme = useTheme() + const [value, setValue] = useState(widget.values[0]) + const render = () => ( + + + {metricName} - {widget.description} + + + {widget.choices.map((c, j) => ( + { + const selected = widget.values.at(j) + if (selected === undefined) { + console.error("Must not reach here.") + return + } + if (e.target.checked) { + setValue(selected) + } + }} + /> + } + label={c} + /> + ))} + + + ) + return { isValid: true, value, render } +} + +export const useSliderWidget = ( + key: string, + widgetType: "user_attr" | "objective", + widget: ObjectiveSliderWidget, + metricName: string +): WidgetState => { + const theme = useTheme() + const [value, setValue] = useState(widget.min) + const render = () => ( + + + {metricName} - {widget.description} + + + { + // @ts-ignore + setValue(e.target.value as number) + }} + defaultValue={widget.min} + min={widget.min} + max={widget.max} + step={widget.step} + marks={ + widget.labels === null || widget.labels.length == 0 + ? true + : widget.labels + } + valueLabelDisplay="auto" + /> + + + ) + return { isValid: true, value, render } +} + +export const useUserAttrRefWidget = ( + key: string, + widget: ObjectiveUserAttrRef, + metricName: string, + trial: Trial +): WidgetState => { + const theme = useTheme() + const value = useMemo(() => { + const attr = trial.user_attrs.find((attr) => attr.key === widget.key) + if (attr === undefined) { + return null + } + const n = Number(attr.value) + if (isNaN(n)) { + return null + } + return n + }, [trial.user_attrs]) + const render = () => ( + + {metricName} + + + ) + return { + isValid: value !== null, + value: value !== null ? value : "", + render, + } } export const ReadonlyObjectiveForm: FC<{ @@ -321,19 +347,18 @@ export const ReadonlyObjectiveForm: FC<{ const theme = useTheme() const getMetricName = (i: number): string => { if (formWidgets.output_type == "objective") { - const n = names.at(i) - if (n !== undefined) { - return n - } - if (directions.length == 1) { - return `Objective` - } else { - return `Objective ${i}` + if (names.at(i) !== undefined) { + return names[i] } + return directions.length == 1 ? "Objective" : `Objective ${i}` } else if (formWidgets.output_type == "user_attr") { - return formWidgets.widgets[i].user_attr_key as string + const key = formWidgets.widgets.at(i)?.user_attr_key + if (key !== undefined) { + return key + } } - return "Unkown metric name" + console.error("Must not reach here") + return "Unknown" } const getValue = (i: number): string | TrialValueNumber => { @@ -441,6 +466,7 @@ export const ReadonlyObjectiveForm: FC<{ ) From 71d667c57bb020ad127daf4a819b431e859c24f0 Mon Sep 17 00:00:00 2001 From: c-bata Date: Wed, 10 May 2023 02:02:25 +0900 Subject: [PATCH 04/10] Add trialsUpdating flags --- optuna_dashboard/ts/action.ts | 15 +++++++++++++++ optuna_dashboard/ts/components/TrialList.tsx | 6 ++++-- optuna_dashboard/ts/state.ts | 12 ++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/optuna_dashboard/ts/action.ts b/optuna_dashboard/ts/action.ts index a97cd522..1017e090 100644 --- a/optuna_dashboard/ts/action.ts +++ b/optuna_dashboard/ts/action.ts @@ -23,6 +23,7 @@ import { isFileUploading, artifactIsAvailable, reloadIntervalState, + trialsUpdatingState, } from "./state" import { getDominatedTrials } from "./dominatedTrials" @@ -45,6 +46,7 @@ export const actionCreator = () => { const [paramImportance, setParamImportance] = useRecoilState(paramImportanceState) const setUploading = useSetRecoilState(isFileUploading) + const setTrialsUpdating = useSetRecoilState(trialsUpdatingState) const setArtifactIsAvailable = useSetRecoilState(artifactIsAvailable) const setStudyDetailState = (studyId: number, study: StudyDetail) => { @@ -60,6 +62,13 @@ export const actionCreator = () => { newStudy.trials = newTrials setStudyDetailState(studyId, newStudy) } + const setTrialUpdating = (trialId: number, updating: boolean) => { + setTrialsUpdating((prev) => { + const newVal = Object.assign({}, prev) + newVal[trialId] = updating + return newVal + }) + } const setTrialNote = (studyId: number, index: number, note: Note) => { const newTrial: Trial = Object.assign( @@ -467,6 +476,7 @@ export const actionCreator = () => { const makeTrialFail = (studyId: number, trialId: number): void => { const message = `id=${trialId}, state=Fail` + setTrialUpdating(trialId, true) tellTrialAPI(trialId, "Fail") .then(() => { const index = studyDetails[studyId].trials.findIndex( @@ -484,6 +494,7 @@ export const actionCreator = () => { }) }) .catch((err) => { + setTrialUpdating(trialId, false) const reason = err.response?.data.reason enqueueSnackbar( `Failed to update trial (${message}). Reason: ${reason}`, @@ -501,6 +512,7 @@ export const actionCreator = () => { values: number[] ): void => { const message = `id=${trialId}, state=Complete, values=${values}` + setTrialUpdating(trialId, true) tellTrialAPI(trialId, "Complete", values) .then(() => { const index = studyDetails[studyId].trials.findIndex( @@ -515,6 +527,7 @@ export const actionCreator = () => { setTrialStateValues(studyId, index, "Complete", values) }) .catch((err) => { + setTrialUpdating(trialId, false) const reason = err.response?.data.reason enqueueSnackbar( `Failed to update trial (${message}). Reason: ${reason}`, @@ -532,6 +545,7 @@ export const actionCreator = () => { user_attrs: { [key: string]: string | number } ): void => { const message = `id=${trialId}, user_attrs=${JSON.stringify(user_attrs)}` + setTrialUpdating(trialId, true) saveTrialUserAttrsAPI(trialId, user_attrs) .then(() => { const index = studyDetails[studyId].trials.findIndex( @@ -549,6 +563,7 @@ export const actionCreator = () => { }) }) .catch((err) => { + setTrialUpdating(trialId, false) const reason = err.response?.data.reason enqueueSnackbar( `Failed to update trial (${message}). Reason: ${reason}`, diff --git a/optuna_dashboard/ts/components/TrialList.tsx b/optuna_dashboard/ts/components/TrialList.tsx index 66031c20..18ce2f2b 100644 --- a/optuna_dashboard/ts/components/TrialList.tsx +++ b/optuna_dashboard/ts/components/TrialList.tsx @@ -39,7 +39,7 @@ import { TrialNote } from "./Note" import { useHistory, useLocation } from "react-router-dom" import ListItemIcon from "@mui/material/ListItemIcon" import { useRecoilValue } from "recoil" -import { artifactIsAvailable } from "../state" +import { artifactIsAvailable, useTrialUpdatingValue } from "../state" import { actionCreator } from "../action" import { useDeleteArtifactDialog } from "./DeleteArtifactDialog" import { ObjectiveForm, ReadonlyObjectiveForm } from "./ObjectiveForm" @@ -147,6 +147,7 @@ const TrialListDetail: FC<{ }> = ({ trial, isBestTrial, directions, objectiveNames, formWidgets }) => { const theme = useTheme() const artifactEnabled = useRecoilValue(artifactIsAvailable) + const formWidgetLoading = useTrialUpdatingValue(trial.trial_id) const startMs = trial.datetime_start?.getTime() const completeMs = trial.datetime_complete?.getTime() @@ -275,6 +276,7 @@ const TrialListDetail: FC<{ cardSx={{ marginBottom: theme.spacing(2) }} /> {trial.state === "Running" && + !formWidgetLoading && directions.length > 0 && formWidgets !== undefined && ( )} - {trial.state === "Complete" && + {(trial.state === "Complete" || formWidgetLoading) && directions.length > 0 && formWidgets !== undefined && ( ({ default: {}, }) +export const trialsUpdatingState = atom<{ + [trialId: string]: boolean +}>({ + key: "trialsUpdating", + default: {}, +}) + export const paramImportanceState = atom({ key: "paramImportance", default: {}, @@ -59,6 +66,11 @@ export const useStudySummaryValue = (studyId: number): StudySummary | null => { return studySummaries.find((s) => s.study_id == studyId) || null } +export const useTrialUpdatingValue = (trialId: number): boolean => { + const updating = useRecoilValue(trialsUpdatingState) + return updating[trialId] || false +} + export const useParamImportanceValue = ( studyId: number ): ParamImportance[][] | null => { From baa618ee2043b144f8c117e72c95d8e91e10448b Mon Sep 17 00:00:00 2001 From: c-bata Date: Wed, 10 May 2023 02:10:11 +0900 Subject: [PATCH 05/10] Refactor ObjectiveForm component --- .../ts/components/ObjectiveForm.tsx | 87 +++++++------------ 1 file changed, 32 insertions(+), 55 deletions(-) diff --git a/optuna_dashboard/ts/components/ObjectiveForm.tsx b/optuna_dashboard/ts/components/ObjectiveForm.tsx index 74a08f18..bdaa572c 100644 --- a/optuna_dashboard/ts/components/ObjectiveForm.tsx +++ b/optuna_dashboard/ts/components/ObjectiveForm.tsx @@ -31,48 +31,19 @@ export const ObjectiveForm: FC<{ const theme = useTheme() const action = actionCreator() - const getMetricName = (i: number): string => { - if (formWidgets.output_type == "objective") { - if (names.at(i) !== undefined) { - return names[i] - } - return directions.length == 1 ? "Objective" : `Objective ${i}` - } else if (formWidgets.output_type == "user_attr") { - const key = formWidgets.widgets.at(i)?.user_attr_key - if (key !== undefined) { - return key - } - } - console.error("Must not reach here") - return "Unknown" - } - const widgetStates = formWidgets.widgets .map((w, i) => { const key = `${formWidgets.output_type}-${i}` + const outputType = formWidgets.output_type + const metricName = getMetricName(formWidgets, names, directions, i) if (w.type === "text") { - return useTextInputWidget( - key, - formWidgets.output_type, - w, - getMetricName(i) - ) + return useTextInputWidget(key, outputType, w, metricName) } else if (w.type === "choice") { - return useChoiceWidget( - key, - formWidgets.output_type, - w, - getMetricName(i) - ) + return useChoiceWidget(key, outputType, w, metricName) } else if (w.type === "slider") { - return useSliderWidget( - key, - formWidgets.output_type, - w, - getMetricName(i) - ) + return useSliderWidget(key, outputType, w, metricName) } else if (w.type === "user_attr") { - return useUserAttrRefWidget(key, w, getMetricName(i), trial) + return useUserAttrRefWidget(key, w, metricName, trial) } console.error("Must not reach here") return undefined @@ -345,22 +316,6 @@ export const ReadonlyObjectiveForm: FC<{ formWidgets: FormWidgets }> = ({ trial, directions, names, formWidgets }) => { const theme = useTheme() - const getMetricName = (i: number): string => { - if (formWidgets.output_type == "objective") { - if (names.at(i) !== undefined) { - return names[i] - } - return directions.length == 1 ? "Objective" : `Objective ${i}` - } else if (formWidgets.output_type == "user_attr") { - const key = formWidgets.widgets.at(i)?.user_attr_key - if (key !== undefined) { - return key - } - } - console.error("Must not reach here") - return "Unknown" - } - const getValue = (i: number): string | TrialValueNumber => { if (formWidgets.output_type === "user_attr") { const widget = formWidgets.widgets[i] as UserAttrFormWidget @@ -397,11 +352,12 @@ export const ReadonlyObjectiveForm: FC<{ > {formWidgets.widgets.map((widget, i) => { const key = `objective-${i}` + const metricName = getMetricName(formWidgets, names, directions, i) if (widget.type === "text") { return ( - {getMetricName(i)} - {widget.description} + {metricName} - {widget.description} - {getMetricName(i)} - {widget.description} + {metricName} - {widget.description} {widget.choices.map((c, j) => ( @@ -438,7 +394,7 @@ export const ReadonlyObjectiveForm: FC<{ return ( - {getMetricName(i)} - {widget.description} + {metricName} - {widget.description} - {getMetricName(i)} + {metricName} ) } + +const getMetricName = ( + formWidgets: FormWidgets, + names: string[], + directions: StudyDirection[], + i: number +): string => { + if (formWidgets.output_type == "objective") { + if (names.at(i) !== undefined) { + return names[i] + } + return directions.length == 1 ? "Objective" : `Objective ${i}` + } else if (formWidgets.output_type == "user_attr") { + const key = formWidgets.widgets.at(i)?.user_attr_key + if (key !== undefined) { + return key + } + } + console.error("Must not reach here") + return "Unknown" +} From 9493775d8c7e2b00ec54008bf2414a031d03ac49 Mon Sep 17 00:00:00 2001 From: c-bata Date: Wed, 10 May 2023 02:35:00 +0900 Subject: [PATCH 06/10] Rename to TrialFormWidgets.tsx --- ...ObjectiveForm.tsx => TrialFormWidgets.tsx} | 359 +++++++++--------- optuna_dashboard/ts/components/TrialList.tsx | 34 +- 2 files changed, 194 insertions(+), 199 deletions(-) rename optuna_dashboard/ts/components/{ObjectiveForm.tsx => TrialFormWidgets.tsx} (56%) diff --git a/optuna_dashboard/ts/components/ObjectiveForm.tsx b/optuna_dashboard/ts/components/TrialFormWidgets.tsx similarity index 56% rename from optuna_dashboard/ts/components/ObjectiveForm.tsx rename to optuna_dashboard/ts/components/TrialFormWidgets.tsx index bdaa572c..c76dd92c 100644 --- a/optuna_dashboard/ts/components/ObjectiveForm.tsx +++ b/optuna_dashboard/ts/components/TrialFormWidgets.tsx @@ -15,6 +15,7 @@ import { } from "@mui/material" import { DebouncedInputTextField } from "./Debounce" import { actionCreator } from "../action" +import { useTrialUpdatingValue } from "../state" type WidgetState = { isValid: boolean @@ -22,12 +23,68 @@ type WidgetState = { render: () => ReactNode } -export const ObjectiveForm: FC<{ +export const TrialFormWidgets: FC<{ trial: Trial + objectiveNames: string[] directions: StudyDirection[] - names: string[] formWidgets: FormWidgets -}> = ({ trial, directions, names, formWidgets }) => { +}> = ({ trial, objectiveNames, directions, formWidgets }) => { + const theme = useTheme() + const formWidgetLoading = useTrialUpdatingValue(trial.trial_id) + const headerText = + formWidgets.output_type === "user_attr" + ? "Set User Attributes Form" + : directions.length > 1 + ? "Set Objective Values Form" + : "Set Objective Value Form" + const widgetNames = formWidgets.widgets.map((widget, i) => { + if (formWidgets.output_type == "objective") { + if (objectiveNames.at(i) !== undefined) { + return objectiveNames[i] + } + return directions.length == 1 ? "Objective" : `Objective ${i}` + } else if (formWidgets.output_type == "user_attr") { + if (widget.type !== "user_attr" && widget.user_attr_key !== undefined) { + return widget.user_attr_key + } + } + console.error("Must not reach here") + return "Unknown" + }) + return ( + <> + + {headerText} + + {trial.state === "Running" && + !formWidgetLoading && + directions.length > 0 && ( + <_FormWidgets + trial={trial} + widgetNames={widgetNames} + formWidgets={formWidgets} + /> + )} + {(trial.state === "Complete" || formWidgetLoading) && + directions.length > 0 && ( + + )} + + ) +} + +const _FormWidgets: FC<{ + trial: Trial + widgetNames: string[] + formWidgets: FormWidgets +}> = ({ trial, widgetNames, formWidgets }) => { const theme = useTheme() const action = actionCreator() @@ -35,15 +92,14 @@ export const ObjectiveForm: FC<{ .map((w, i) => { const key = `${formWidgets.output_type}-${i}` const outputType = formWidgets.output_type - const metricName = getMetricName(formWidgets, names, directions, i) if (w.type === "text") { - return useTextInputWidget(key, outputType, w, metricName) + return useTextInputWidget(key, outputType, w, widgetNames[i]) } else if (w.type === "choice") { - return useChoiceWidget(key, outputType, w, metricName) + return useChoiceWidget(key, outputType, w, widgetNames[i]) } else if (w.type === "slider") { - return useSliderWidget(key, outputType, w, metricName) + return useSliderWidget(key, outputType, w, widgetNames[i]) } else if (w.type === "user_attr") { - return useUserAttrRefWidget(key, w, metricName, trial) + return useUserAttrRefWidget(key, w, widgetNames[i], trial) } console.error("Must not reach here") return undefined @@ -60,7 +116,7 @@ export const ObjectiveForm: FC<{ const values = widgetStates.map((ws) => ws.value) if (formWidgets.output_type == "objective") { const filtered = values.filter((v): v is number => v !== null) - if (filtered.length !== directions.length) { + if (filtered.length !== formWidgets.widgets.length) { return } action.makeTrialComplete(trial.study_id, trial.trial_id, filtered) @@ -75,62 +131,47 @@ export const ObjectiveForm: FC<{ } } - const headerText = - formWidgets.output_type === "user_attr" - ? "Set User Attributes Form" - : directions.length > 1 - ? "Set Objective Values Form" - : "Set Objective Value Form" - return ( - <> - + - {headerText} - - - ws.render())} + - {widgetStates.map((ws) => ws.render())} - + Submit + + + - - - - - - + Fail Trial + + + + ) } @@ -309,12 +350,11 @@ export const useUserAttrRefWidget = ( } } -export const ReadonlyObjectiveForm: FC<{ +const ReadonlyFormWidgets: FC<{ trial: Trial - directions: StudyDirection[] - names: string[] + widgetNames: string[] formWidgets: FormWidgets -}> = ({ trial, directions, names, formWidgets }) => { +}> = ({ trial, widgetNames, formWidgets }) => { const theme = useTheme() const getValue = (i: number): string | TrialValueNumber => { if (formWidgets.output_type === "user_attr") { @@ -333,125 +373,94 @@ export const ReadonlyObjectiveForm: FC<{ } return ( - <> - + - {directions.length > 1 ? "Set Objective Values" : "Set Objective Value"} - - - - {formWidgets.widgets.map((widget, i) => { - const key = `objective-${i}` - const metricName = getMetricName(formWidgets, names, directions, i) - if (widget.type === "text") { - return ( - - - {metricName} - {widget.description} - - - - ) - } else if (widget.type === "choice") { - return ( - - - {metricName} - {widget.description} - - - {widget.choices.map((c, j) => ( - - } - label={c} - disabled - /> - ))} - - - ) - } else if (widget.type === "slider") { - const value = trial.values?.at(i) - return ( - - - {metricName} - {widget.description} - - - { + const key = `objective-${i}` + const widgetName = widgetNames[i] + if (widget.type === "text") { + return ( + + + {widgetName} - {widget.description} + + + + ) + } else if (widget.type === "choice") { + return ( + + + {widgetName} - {widget.description} + + + {widget.choices.map((c, j) => ( + } - min={widget.min} - max={widget.max} - step={widget.step} - marks={ - widget.labels === null || widget.labels.length == 0 - ? true - : widget.labels - } - valueLabelDisplay="auto" + label={c} disabled /> - - - ) - } else if (widget.type === "user_attr") { - return ( - - {metricName} - + + ) + } else if (widget.type === "slider") { + const value = trial.values?.at(i) + return ( + + + {widgetName} - {widget.description} + + + - - ) - } - return null - })} - - - + + + ) + } else if (widget.type === "user_attr") { + return ( + + {widgetName} + + + ) + } + return null + })} + + ) } - -const getMetricName = ( - formWidgets: FormWidgets, - names: string[], - directions: StudyDirection[], - i: number -): string => { - if (formWidgets.output_type == "objective") { - if (names.at(i) !== undefined) { - return names[i] - } - return directions.length == 1 ? "Objective" : `Objective ${i}` - } else if (formWidgets.output_type == "user_attr") { - const key = formWidgets.widgets.at(i)?.user_attr_key - if (key !== undefined) { - return key - } - } - console.error("Must not reach here") - return "Unknown" -} diff --git a/optuna_dashboard/ts/components/TrialList.tsx b/optuna_dashboard/ts/components/TrialList.tsx index 18ce2f2b..547da40c 100644 --- a/optuna_dashboard/ts/components/TrialList.tsx +++ b/optuna_dashboard/ts/components/TrialList.tsx @@ -39,10 +39,10 @@ import { TrialNote } from "./Note" import { useHistory, useLocation } from "react-router-dom" import ListItemIcon from "@mui/material/ListItemIcon" import { useRecoilValue } from "recoil" -import { artifactIsAvailable, useTrialUpdatingValue } from "../state" +import { artifactIsAvailable } from "../state" import { actionCreator } from "../action" import { useDeleteArtifactDialog } from "./DeleteArtifactDialog" -import { ObjectiveForm, ReadonlyObjectiveForm } from "./ObjectiveForm" +import { TrialFormWidgets } from "./TrialFormWidgets" const states: TrialState[] = [ "Complete", @@ -147,7 +147,6 @@ const TrialListDetail: FC<{ }> = ({ trial, isBestTrial, directions, objectiveNames, formWidgets }) => { const theme = useTheme() const artifactEnabled = useRecoilValue(artifactIsAvailable) - const formWidgetLoading = useTrialUpdatingValue(trial.trial_id) const startMs = trial.datetime_start?.getTime() const completeMs = trial.datetime_complete?.getTime() @@ -275,27 +274,14 @@ const TrialListDetail: FC<{ latestNote={trial.note} cardSx={{ marginBottom: theme.spacing(2) }} /> - {trial.state === "Running" && - !formWidgetLoading && - directions.length > 0 && - formWidgets !== undefined && ( - - )} - {(trial.state === "Complete" || formWidgetLoading) && - directions.length > 0 && - formWidgets !== undefined && ( - - )} + {formWidgets !== undefined && ( + + )} Date: Wed, 10 May 2023 09:28:11 +0900 Subject: [PATCH 07/10] Refactor TrialFormWidgets --- .../ts/components/TrialFormWidgets.tsx | 44 ++++++++++--------- optuna_dashboard/ts/components/TrialList.tsx | 14 +++--- 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/optuna_dashboard/ts/components/TrialFormWidgets.tsx b/optuna_dashboard/ts/components/TrialFormWidgets.tsx index c76dd92c..14609fe5 100644 --- a/optuna_dashboard/ts/components/TrialFormWidgets.tsx +++ b/optuna_dashboard/ts/components/TrialFormWidgets.tsx @@ -27,10 +27,17 @@ export const TrialFormWidgets: FC<{ trial: Trial objectiveNames: string[] directions: StudyDirection[] - formWidgets: FormWidgets + formWidgets?: FormWidgets }> = ({ trial, objectiveNames, directions, formWidgets }) => { + if ( + formWidgets === undefined || + trial.state === "Pruned" || + trial.state === "Fail" + ) { + return null + } const theme = useTheme() - const formWidgetLoading = useTrialUpdatingValue(trial.trial_id) + const trialNowUpdating = useTrialUpdatingValue(trial.trial_id) const headerText = formWidgets.output_type === "user_attr" ? "Set User Attributes Form" @@ -51,6 +58,7 @@ export const TrialFormWidgets: FC<{ console.error("Must not reach here") return "Unknown" }) + return ( <> {headerText} - {trial.state === "Running" && - !formWidgetLoading && - directions.length > 0 && ( - <_FormWidgets - trial={trial} - widgetNames={widgetNames} - formWidgets={formWidgets} - /> - )} - {(trial.state === "Complete" || formWidgetLoading) && - directions.length > 0 && ( - - )} + {trial.state === "Running" && !trialNowUpdating ? ( + + ) : ( + + )} ) } -const _FormWidgets: FC<{ +const UpdatableFormWidgets: FC<{ trial: Trial widgetNames: string[] formWidgets: FormWidgets diff --git a/optuna_dashboard/ts/components/TrialList.tsx b/optuna_dashboard/ts/components/TrialList.tsx index 547da40c..6ccfa516 100644 --- a/optuna_dashboard/ts/components/TrialList.tsx +++ b/optuna_dashboard/ts/components/TrialList.tsx @@ -274,14 +274,12 @@ const TrialListDetail: FC<{ latestNote={trial.note} cardSx={{ marginBottom: theme.spacing(2) }} /> - {formWidgets !== undefined && ( - - )} + Date: Wed, 10 May 2023 09:37:48 +0900 Subject: [PATCH 08/10] Update docstring of SliderWidget --- optuna_dashboard/_form_widget.py | 1 + 1 file changed, 1 insertion(+) diff --git a/optuna_dashboard/_form_widget.py b/optuna_dashboard/_form_widget.py index 34d4336b..ff1b186e 100644 --- a/optuna_dashboard/_form_widget.py +++ b/optuna_dashboard/_form_widget.py @@ -185,6 +185,7 @@ class TextInputWidget: description: A description of the text input field. user_attr_key: The key used by `register_user_attr_form_widgets`. Form output is saved as `trial.user_attrs[user_attr_key]`. Defaults to None. + optional: If True, an empty string is acceptable. Example: .. code-block:: python From 31339db3673b34482c677a686000049464931039 Mon Sep 17 00:00:00 2001 From: c-bata Date: Wed, 10 May 2023 09:38:39 +0900 Subject: [PATCH 09/10] Fix lint errors --- optuna_dashboard/_form_widget.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/optuna_dashboard/_form_widget.py b/optuna_dashboard/_form_widget.py index ff1b186e..03b29f66 100644 --- a/optuna_dashboard/_form_widget.py +++ b/optuna_dashboard/_form_widget.py @@ -46,7 +46,12 @@ if TYPE_CHECKING: ) TextInputWidgetJSON = TypedDict( "TextInputWidgetJSON", - {"type": Literal["text"], "description": Optional[str], "user_attr_key": Optional[str], "optional": bool}, + { + "type": Literal["text"], + "description": Optional[str], + "user_attr_key": Optional[str], + "optional": bool, + }, ) UserAttrRefJSON = TypedDict("UserAttrRefJSON", {"type": Literal["user_attr"], "key": str}) FormWidgetJSON = TypedDict( @@ -351,9 +356,7 @@ def register_objective_form_widgets( not isinstance(w, ObjectiveUserAttrRef) and w.user_attr_key is not None for w in widgets ): warnings.warn("`user_attr_key` specified, but it will not be used.") - if any( - isinstance(w, TextInputWidget) and w.optional for w in widgets - ): + if any(isinstance(w, TextInputWidget) and w.optional for w in widgets): raise ValueError("TextInputWidget.optional must be False.") form_widgets: FormWidgetJSON = { "output_type": "objective", From 16a6265d64a6baa61ed7b685bdd77c14f1099fca Mon Sep 17 00:00:00 2001 From: c-bata Date: Wed, 10 May 2023 09:51:27 +0900 Subject: [PATCH 10/10] Fix a broken test --- python_tests/test_serializers.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/python_tests/test_serializers.py b/python_tests/test_serializers.py index 131013c9..4ea8c9bd 100644 --- a/python_tests/test_serializers.py +++ b/python_tests/test_serializers.py @@ -10,17 +10,6 @@ class SerializeAttrsTestCase(TestCase): serialized = serialize_attrs({"bytes": b"This is a bytes object."}) self.assertEqual(serialized[0]["value"], "") - def test_serialize_string(self) -> None: - for length in [1000, 1024, 1100]: - with self.subTest(f"length: {length}"): - value = "a" * length - serialized = serialize_attrs( - { - "key": value, - } - ) - self.assertLessEqual(len(serialized[0]["value"]), 1024) - def test_serialize_dict(self) -> None: serialized = serialize_attrs( {