Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Trial Selection page #996

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

hrntsm
Copy link
Collaborator

@hrntsm hrntsm commented Nov 27, 2024

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.

The current Optuna Dashboard has a full range of analysis functions for optimization, but I feel that the functions related to solution selection are still insufficient, and I would like to strengthen them.

Especially in the case of multi-objective optimization, the Pareto front alone has a very large number of individuals, and it is difficult to choose which one to select.
Therefore, I would like to provide a function to assist in the selection process.

@hrntsm
Copy link
Collaborator Author

hrntsm commented Nov 27, 2024

Currently, the graph is implemented in a file copied from an existing file with the name “TC” (trial compare) added, but it is hoped that this can eventually be reflected in the existing file from the standpoint of code management.

Copy link

codecov bot commented Nov 27, 2024

Codecov Report

Attention: Patch coverage is 35.71429% with 9 lines in your changes missing coverage. Please review.

Project coverage is 69.79%. Comparing base (44e822f) to head (6ade08b).
Report is 26 commits behind head on main.

Files with missing lines Patch % Lines
optuna_dashboard/_app.py 35.71% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #996      +/-   ##
==========================================
- Coverage   70.00%   69.79%   -0.22%     
==========================================
  Files          35       35              
  Lines        2387     2400      +13     
==========================================
+ Hits         1671     1675       +4     
- Misses        716      725       +9     

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

@c-bata c-bata self-assigned this Nov 29, 2024
@hrntsm
Copy link
Collaborator Author

hrntsm commented Dec 5, 2024

Added Include dominated trials toggle to make it easier to select Pareto front trials.

include dominated only pareto front
Screenshot 2024-12-05 at 18 05 51 Screenshot 2024-12-05 at 17 57 40

@c-bata
Copy link
Member

c-bata commented Dec 6, 2024

Thank you for your pull request! Regarding the CI failure in the typescript-checks, I fixed the error in #999.

@hrntsm
Copy link
Collaborator Author

hrntsm commented Dec 6, 2024

Add Show Experimental Feature toggle in Settings. This toggle to show/hide this PR feature icon in AppDrawer.

Now that I have achieved the minimal implementation that I had envisioned, I have finished implementing the functionality in this PR and plan to integrate the separate graph files and other files into the existing code and submit a request for review.

I plan to introduce some more updates after this PR.

@hrntsm hrntsm marked this pull request as ready for review December 18, 2024 02:32
@hrntsm
Copy link
Collaborator Author

hrntsm commented Dec 18, 2024

@c-bata
The codes are cleaned up and ready for review.
Could you please review it?

@hrntsm
Copy link
Collaborator Author

hrntsm commented Jan 12, 2025

Artifact views have been added to the Selection page. Artifact indexes can be specified and displayed.
To give an approximate idea of how good or bad an Artifact is, the border of the Artifact is colored to indicate whether the target's object value is greater than or less than its value.

The Show Artifacts toggle controls the activation of the display, which is off by default.

Screenshot 2025-01-12 at 14 06 49 Screenshot 2025-01-12 at 13 55 52

@hrntsm
Copy link
Collaborator Author

hrntsm commented Jan 20, 2025

@c-bata
Could you please review this when you have time as all CI's have been passed.

@porink0424 porink0424 self-assigned this Jan 29, 2025
Copy link
Collaborator

@porink0424 porink0424 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR!
I've left some comments, PTAL.

optuna_dashboard/ts/components/AppDrawer.tsx Outdated Show resolved Hide resolved
tslib/react/src/components/TrialTable.tsx Outdated Show resolved Hide resolved
tslib/react/src/utils/trialFilter.ts Outdated Show resolved Hide resolved
optuna_dashboard/ts/components/GraphParetoFront.tsx Outdated Show resolved Hide resolved
optuna_dashboard/ts/components/TrialSelection.tsx Outdated Show resolved Hide resolved
optuna_dashboard/ts/components/TrialSelection.tsx Outdated Show resolved Hide resolved
@hrntsm
Copy link
Collaborator Author

hrntsm commented Jan 31, 2025

@porink0424
Thank you for your comment.
I have addressed the comments, please check back.

Copy link
Collaborator

@porink0424 porink0424 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hrntsm
Thank you for quickly making the code fixes based on my review,
I've checked the modifications and left some replies, so please take a look.

Also, it seems the linting check has failed, so please take a look at that as well.

@hrntsm
Copy link
Collaborator Author

hrntsm commented Feb 5, 2025

@porink0424
The lint check is Fail in the file that was not changed in this PR, and I don't think it is directly related to this PR.
Is it preferable to create a separate PR to address this issue?

@hrntsm
Copy link
Collaborator Author

hrntsm commented Feb 6, 2025

As for lint, it was a small change and has been corrected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants