Add plot to redirect click event #625

hrntsm · 2023-09-22T12:22:58Z

Contributor License Agreement

This repository (optuna-dashboard) and Goptuna share common code.
This pull request may therefore be ported to Goptuna.
Make sure that you understand the consequences concerning licenses and check the box below if you accept the term before creating this pull request.

I agree this patch may be ported to Goptuna by other Goptuna contributors.

Reference Issues/PRs

What does this implement/fix? Explain your changes.

To make it easier to check the results, it make possible to jump to the details of the trial from the history and pareto-front plots.

Screen.Recording.2023-09-22.at.21.18.13.mov

codecov · 2023-09-22T12:25:10Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1dd3849) 56.40% compared to head (db2f631) 62.45%.
Report is 339 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #625      +/-   ##
==========================================
+ Coverage   56.40%   62.45%   +6.04%     
==========================================
  Files          35       35              
  Lines        2193     2224      +31     
==========================================
+ Hits         1237     1389     +152     
+ Misses        956      835     -121

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

keisuke-umezawa

Alghouth I left some nits comment, It works well in my local.

keisuke-umezawa · 2023-09-23T07:45:26Z

optuna_dashboard/ts/components/GraphHistory.tsx

+      element.on("plotly_click", function (data) {
+        const link =
+          URL_PREFIX +
+          `/studies/${studies[0].id}}/trials?numbers=${data.points[0].x}`


In this ref, it seems that we can get curveNumber from data.

How about replace studies[0] to studies[Math.floor(data.points[0].curveNumber / 2)]? In that case, it also work with studies.length >=1 .

keisuke-umezawa · 2023-09-23T07:45:58Z

optuna_dashboard/ts/components/GraphHistory.tsx

+      element.on("plotly_click", function (data) {
+        const link =
+          URL_PREFIX +
+          `/studies/${studies[0].id}}/trials?numbers=${data.points[0].x}`


[nits] It seems to have redundant } here.

As a result, the url became something like http://localhost:8080/dashboard/studies/0%7D/trials?numbers=0, where %7D comes from }.

keisuke-umezawa · 2023-09-23T07:46:12Z

optuna_dashboard/ts/components/GraphParetoFront.tsx

+          )
+          const link =
+            URL_PREFIX +
+            `/studies/${study.id}}/trials?numbers=${plotTextInfo.number}`


[nits] It seems to have redundant } here.

hrntsm · 2023-09-24T13:01:10Z

Thanks for the review @keisuke-umezawa .
I have fixed the points you suggested.

hrntsm · 2023-09-24T13:03:00Z

optuna_dashboard/ts/components/GraphHistory.tsx

+    if (element != null && studies.length >= 1) {
+      // @ts-ignore
+      element.on("plotly_click", function (data) {
+        if (data.points[0].data.mode !== "lines") {


As an addition, the x coordinate of the point was used for the history redirection, which also worked for the best value line.
I want to redirect only when the trial point itself is clicked, so it will not work when the line is clicked like below.

c-bata

Thank you for your pull request! I left a suggestion.

c-bata · 2023-09-29T01:36:34Z

optuna_dashboard/ts/components/GraphHistory.tsx

+          const link =
+            URL_PREFIX +
+            `/studies/${
+              studies[Math.floor(data.points[0].curveNumber / 2)].id
+            }/trials?numbers=${data.points[0].x}`
+          window.location.href = link


Could you use useNavigate() to avoid the page refresh?

optuna-dashboard/optuna_dashboard/ts/components/TrialList.tsx

Line 333 in 2f99e3b

const navigate = useNavigate()

Suggested change

const link =

URL_PREFIX +

`/studies/${

studies[Math.floor(data.points[0].curveNumber / 2)].id

}/trials?numbers=${data.points[0].x}`

window.location.href = link

navigate(

URL_PREFIX +

`/studies/${

studies[Math.floor(data.points[0].curveNumber / 2)].id

}/trials?numbers=${data.points[0].x}`

)

c-bata · 2023-09-29T01:36:54Z

optuna_dashboard/ts/components/GraphParetoFront.tsx

+          const link =
+            URL_PREFIX +
+            `/studies/${study.id}/trials?numbers=${plotTextInfo.number}`
+          window.location.href = link


hrntsm · 2023-09-29T05:22:40Z

optuna_dashboard/ts/components/GraphHistory.tsx

+          if (data.points[0].data.name.includes("Infeasible Trial of")) {
+            const studyInfo: { id: number; name: string }[] = []
+            studies.forEach((study) => {
+              studyInfo.push({ id: study.id, name: study.name })
+            })
+            const dataPointStudyName = data.points[0].data.name.replace(
+              "Infeasible Trial of ",
+              ""
+            )
+            const targetId = studyInfo.find((s) => s.name === dataPointStudyName)?.id
+            if (targetId !== undefined) {
+              studyId = targetId
+            }


It did not work for Infeasible trials and has been corrected.
StudyId is identified from the name string of the point, but if you have a better way, please comment.

One solution would be to give each plot a little more information, like a Pareto chart.
If this is a good solution, I will create another PR on this and as soon as that it is merged, I will reflect it in this PR.

keisuke-umezawa · 2023-10-26T03:05:05Z

@c-bata
When you have a time, could you check it again?

github-actions · 2024-01-11T23:01:27Z

This pull request has not seen any recent activity.

Since my requested changes were applied.

c-bata · 2024-02-02T08:28:41Z

@porink0424 Could you review this PR?

porink0424

LGTM,
although I found some (tiny) format problems, I am going to make a follow-up PR.

I think this PR itself can be merged.

hrntsm added 3 commits September 22, 2023 21:05

Add redirect to pareto front

02917ab

Add redirect to history

186f7ac

Remove console.log output

3e82230

keisuke-umezawa approved these changes Sep 23, 2023

View reviewed changes

hrntsm added 3 commits September 24, 2023 21:43

Remove redundant '}'

9d510fb

Update redirect to work with multi studies

37f0709

Update to not redirect when selecting lines

475eaa7

hrntsm commented Sep 24, 2023

View reviewed changes

c-bata self-assigned this Sep 25, 2023

c-bata previously requested changes Sep 29, 2023

View reviewed changes

hrntsm added 2 commits September 29, 2023 13:06

Update to use useNavigate

d1d5f3c

Fix infeasible point redirect

563775e

hrntsm commented Sep 29, 2023

View reviewed changes

Apply formatter

db2f631

keisuke-umezawa requested a review from c-bata November 12, 2023 07:37

github-actions bot added the stale for stale bot label Jan 11, 2024

keisuke-umezawa added the no-stale Exempt from stale bot label Jan 15, 2024

github-actions bot removed the stale for stale bot label Jan 15, 2024

c-bata removed the no-stale Exempt from stale bot label Feb 2, 2024

porink0424 approved these changes Feb 2, 2024

View reviewed changes

c-bata merged commit fc0d52c into optuna:main Feb 2, 2024
9 checks passed

porink0424 mentioned this pull request Feb 2, 2024

Fix some tiny format problems #790

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add plot to redirect click event #625

Add plot to redirect click event #625

hrntsm commented Sep 22, 2023 •

edited

Loading

codecov bot commented Sep 22, 2023 •

edited

Loading

keisuke-umezawa left a comment

keisuke-umezawa Sep 23, 2023

keisuke-umezawa Sep 23, 2023

keisuke-umezawa Sep 23, 2023

keisuke-umezawa Sep 23, 2023

hrntsm commented Sep 24, 2023

hrntsm Sep 24, 2023

c-bata left a comment

c-bata Sep 29, 2023

c-bata Sep 29, 2023

hrntsm Sep 29, 2023

hrntsm Oct 30, 2023

keisuke-umezawa commented Oct 26, 2023

github-actions bot commented Jan 11, 2024

c-bata commented Feb 2, 2024

porink0424 left a comment

Add plot to redirect click event #625

Add plot to redirect click event #625

Conversation

hrntsm commented Sep 22, 2023 • edited Loading

Contributor License Agreement

Reference Issues/PRs

What does this implement/fix? Explain your changes.

codecov bot commented Sep 22, 2023 • edited Loading

Codecov Report

keisuke-umezawa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hrntsm commented Sep 24, 2023

Choose a reason for hiding this comment

c-bata left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keisuke-umezawa commented Oct 26, 2023

github-actions bot commented Jan 11, 2024

c-bata commented Feb 2, 2024

porink0424 left a comment

Choose a reason for hiding this comment

hrntsm commented Sep 22, 2023 •

edited

Loading

codecov bot commented Sep 22, 2023 •

edited

Loading