Merge pull request #52 from optuna/refactor-graph-slice2

Remove redundant computations for GraphSlice

optuna_dashboard/static/components/GraphSlice.tsx

@@ -22,25 +22,37 @@ const useStyles = makeStyles((theme: Theme) =>
   })
 )
 
+const getParamNames = (trials: Trial[]): string[] => {
+  const paramSet = new Set<string>(
+    ...trials.map<string[]>((t) => t.params.map((p) => p.name))
+  )
+  return Array.from(paramSet)
+}
+
 export const GraphSlice: FC<{
   study: StudyDetail | null
 }> = ({ study = null }) => {
-  const trials: Trial[] = study !== null ? study.trials : []
-  const filteredTrials = trials.filter(
-    (t) => t.state === "Complete" || t.state === "Pruned"
-  )
-
-  let paramNames = new Set<string>(trials[0].params.map((p) => p.name))
-  filteredTrials.forEach((t) => {
-    paramNames = new Set<string>(
-      t.params.filter((p) => paramNames.has(p.name)).map((p) => p.name)
-    )
-  })
-  const paramnames = Array.from(paramNames)
-
   const classes = useStyles()
+  const trials: Trial[] = study !== null ? study.trials : []
+  const [paramNames, setParamNames] = useState<string[]>([])
   const [objectiveId, setObjectiveId] = useState<number>(0)
-  const [xAxis, setXAxis] = useState<string>(paramnames[0])
+  const [selected, setSelected] = useState<string | null>(null)
+
+  useEffect(() => {
+    if (trials.length === 0 || paramNames.length !== 0) {
+      return
+    }
+
+    const p = getParamNames(trials)
+    setParamNames(p)
+    if (selected === null && p.length !== 0) {
+      setSelected(p[0])
+    }
+  }, [trials])
+
+  useEffect(() => {
+    plotSlice(trials, objectiveId, selected)
+  }, [trials, objectiveId, selected])
 
   const handleObjectiveChange = (
     event: React.ChangeEvent<{ value: unknown }>
@@ -48,16 +60,10 @@ export const GraphSlice: FC<{
     setObjectiveId(event.target.value as number)
   }
 
-  const handleXAxisChange = (e: ChangeEvent<{ value: unknown }>) => {
-    setXAxis(e.target.value as string)
+  const handleSelectedParam = (e: ChangeEvent<{ value: unknown }>) => {
+    setSelected(e.target.value as string)
   }
 
-  useEffect(() => {
-    if (study != null) {
-      plotSlice(study, objectiveId, xAxis)
-    }
-  }, [study, objectiveId, xAxis])
-
   return (
     <Grid container direction="row">
       <Grid item xs={3}>
@@ -74,16 +80,18 @@ export const GraphSlice: FC<{
               </Select>
             </FormControl>
           ) : null}
-          <FormControl component="fieldset" className={classes.formControl}>
-            <InputLabel id="parameter">Parameter</InputLabel>
-            <Select value={xAxis} onChange={handleXAxisChange}>
-              {paramnames.map((x) => (
-                <MenuItem value={x} key={x}>
-                  {x}
-                </MenuItem>
-              ))}
-            </Select>
-          </FormControl>
+          {paramNames.length !== 0 && selected !== null ? (
+            <FormControl component="fieldset" className={classes.formControl}>
+              <InputLabel id="parameter">Parameter</InputLabel>
+              <Select value={selected} onChange={handleSelectedParam}>
+                {paramNames.map((p, i) => (
+                  <MenuItem value={p} key={i}>
+                    {p}
+                  </MenuItem>
+                ))}
+              </Select>
+            </FormControl>
+          ) : null}
         </Grid>
       </Grid>
       <Grid item xs={6}>
@@ -93,7 +101,11 @@ export const GraphSlice: FC<{
   )
 }
 
-const plotSlice = (study: StudyDetail, objectiveId: number, xAxis: string) => {
+const plotSlice = (
+  trials: Trial[],
+  objectiveId: number,
+  selected: string | null
+) => {
   if (document.getElementById(plotDomId) === null) {
     return
   }
@@ -103,143 +115,107 @@ const plotSlice = (study: StudyDetail, objectiveId: number, xAxis: string) => {
     margin: {
       l: 50,
       r: 50,
-      b: 0,
     },
+    xaxis: {
+      title: selected || "",
+      zerolinecolor: "#f2f5fa",
+      zerolinewidth: 1.5,
+      linecolor: "#f2f5fa",
+      linewidth: 5,
+      gridcolor: "#f2f5fa",
+      gridwidth: 1,
+    },
+    yaxis: {
+      title: "Objective Values",
+      zerolinecolor: "#f2f5fa",
+      zerolinewidth: 2,
+      linecolor: "#f2f5fa",
+      linewidth: 5,
+      gridcolor: "#f2f5fa",
+      gridwidth: 1,
+    },
+    plot_bgcolor: "#E5ecf6",
+    showlegend: false,
   }
 
-  const trials: Trial[] = study !== null ? study.trials : []
-  if (trials.length === 0) {
+  const filteredTrials = trials.filter(
+    (t) =>
+      (t.state === "Complete" || t.state === "Pruned") &&
+      t.params.find((p) => p.name == selected) !== undefined
+  )
+
+  if (filteredTrials.length === 0 || selected === null) {
     plotly.react(plotDomId, [], layout)
     return
   }
 
-  const filteredTrials = trials.filter(
-    (t) => t.state === "Complete" || t.state === "Pruned"
-  )
-
-  let paramNames = new Set<string>(trials[0].params.map((p) => p.name))
-  filteredTrials.forEach((t) => {
-    paramNames = new Set<string>(
-      t.params.filter((p) => paramNames.has(p.name)).map((p) => p.name)
-    )
-  })
-
   const objectiveValues: number[] = filteredTrials.map(
     (t) => t.values![objectiveId]
   )
+  const valueStrings = filteredTrials.map((t) => {
+    return t.params.find((p) => p.name == selected)!.value
+  })
 
-  if (paramNames.size === 0) {
-    plotly.react(plotDomId, [])
-    return
-  } else {
-    let trace: Partial<plotly.PlotData>[] = [
+  const isnum = valueStrings.every((v) => {
+    return !isNaN(parseFloat(v))
+  })
+  if (isnum) {
+    const valuesNum: number[] = valueStrings.map((v) => parseFloat(v))
+    const trace: plotly.Data[] = [
       {
         type: "scatter",
-        x: [],
-        y: [],
+        x: valuesNum,
+        y: objectiveValues,
         mode: "markers",
-        xaxis: "x",
+        xaxis: selected,
         marker: {
           color: "#185799",
         },
       },
     ]
-    const updateLayout: Partial<plotly.Layout> = {
-      title: "Slice",
-      margin: {
-        l: 50,
-        r: 50,
-      },
-      xaxis: {
-        title: "x",
-        zerolinecolor: "#f2f5fa",
-        zerolinewidth: 1.5,
-        linecolor: "#f2f5fa",
-        linewidth: 5,
-        gridcolor: "#f2f5fa",
-        gridwidth: 1,
-      },
-      yaxis: {
-        title: "Objective Values",
-        zerolinecolor: "#f2f5fa",
-        zerolinewidth: 2,
-        linecolor: "#f2f5fa",
-        linewidth: 5,
-        gridcolor: "#f2f5fa",
-        gridwidth: 1,
-      },
-      plot_bgcolor: "#E5ecf6",
-      showlegend: false,
+    layout["xaxis"] = {
+      title: selected,
+      zerolinecolor: "#f2f5fa",
+      zerolinewidth: 1.5,
+      linecolor: "#f2f5fa",
+      linewidth: 5,
+      gridcolor: "#f2f5fa",
+      gridwidth: 1,
     }
-    paramNames.forEach((paramName) => {
-      const valueStrings = filteredTrials.map((t) => {
-        const param = t.params.find((p) => p.name == paramName)
-        return param!.value
-      })
-      const isnum = valueStrings.every((v) => {
-        return !isNaN(parseFloat(v))
-      })
-      if (paramName === xAxis) {
-        if (isnum) {
-          const valuesNum: number[] = valueStrings.map((v) => parseFloat(v))
-          trace = [
-            {
-              type: "scatter",
-              x: valuesNum,
-              y: objectiveValues,
-              mode: "markers",
-              xaxis: paramName,
-              marker: {
-                color: "#185799",
-              },
-            },
-          ]
-          updateLayout["xaxis"] = {
-            title: paramName,
-            zerolinecolor: "#f2f5fa",
-            zerolinewidth: 1.5,
-            linecolor: "#f2f5fa",
-            linewidth: 5,
-            gridcolor: "#f2f5fa",
-            gridwidth: 1,
-          }
-          plotly.react(plotDomId, trace, updateLayout)
-        } else {
-          const vocabSet = new Set<string>(valueStrings)
-          const vocabArr = Array.from<string>(vocabSet)
-          const valuesCategorical: number[] = valueStrings.map((v) =>
-            vocabArr.findIndex((vocab) => v === vocab)
-          )
-          const tickvals: number[] = vocabArr.map((v, i) => i)
-          trace = [
-            {
-              type: "scatter",
-              x: valuesCategorical,
-              y: objectiveValues,
-              mode: "markers",
-              // xaxis: paramName,
-              marker: {
-                color: "#185799",
-              },
-            },
-          ]
-          updateLayout["xaxis"] = {
-            title: paramName,
-            zerolinecolor: "#f2f5fa",
-            zerolinewidth: 1.5,
-            linecolor: "#f2f5fa",
-            linewidth: 5,
-            gridcolor: "#f2f5fa",
-            gridwidth: 1,
-            tickfont: {
-              color: "#000000",
-            },
-            tickvals: tickvals,
-            ticktext: vocabArr,
-          }
-          plotly.react(plotDomId, trace, updateLayout)
-        }
-      }
-    })
+    plotly.react(plotDomId, trace, layout)
+  } else {
+    const vocabSet = new Set<string>(valueStrings)
+    const vocabArr = Array.from<string>(vocabSet)
+    const valuesCategorical: number[] = valueStrings.map((v) =>
+      vocabArr.findIndex((vocab) => v === vocab)
+    )
+    const tickvals: number[] = vocabArr.map((v, i) => i)
+    const trace: plotly.Data[] = [
+      {
+        type: "scatter",
+        x: valuesCategorical,
+        y: objectiveValues,
+        mode: "markers",
+        // xaxis: paramName,
+        marker: {
+          color: "#185799",
+        },
+      },
+    ]
+    layout["xaxis"] = {
+      title: selected,
+      zerolinecolor: "#f2f5fa",
+      zerolinewidth: 1.5,
+      linecolor: "#f2f5fa",
+      linewidth: 5,
+      gridcolor: "#f2f5fa",
+      gridwidth: 1,
+      tickfont: {
+        color: "#000000",
+      },
+      tickvals: tickvals,
+      ticktext: vocabArr,
+    }
+    plotly.react(plotDomId, trace, layout)
   }
 }