Skip to content

Update the documentation string of GPSampler#6081

Merged
y0z merged 12 commits intooptuna:masterfrom
nabenabe0928:update-doc-string-of-gp-sampler
May 23, 2025
Merged

Update the documentation string of GPSampler#6081
y0z merged 12 commits intooptuna:masterfrom
nabenabe0928:update-doc-string-of-gp-sampler

Conversation

@nabenabe0928
Copy link
Copy Markdown
Contributor

Motivation

Since the doc-string of GPSampler is outdated, I updated the documentation accordingly:

Description of the changes

  • Update and enhance the documentation.

@nabenabe0928 nabenabe0928 marked this pull request as ready for review May 13, 2025 07:53
@c-bata
Copy link
Copy Markdown
Member

c-bata commented May 15, 2025

@y0z @kAIto47802 Could you review this PR?

@c-bata c-bata added the document Documentation related. label May 15, 2025
@HideakiImamura
Copy link
Copy Markdown
Member

@sawa3030 Could you review this PR?

@nabenabe0928 nabenabe0928 marked this pull request as draft May 16, 2025 07:40
@kAIto47802
Copy link
Copy Markdown
Collaborator

Thank you for your PR. I've reviewed, leaving some comments. PTAL :octocat:

@nabenabe0928 nabenabe0928 marked this pull request as ready for review May 16, 2025 17:37
@nabenabe0928 nabenabe0928 force-pushed the update-doc-string-of-gp-sampler branch from 6f97896 to ffc253f Compare May 16, 2025 17:37
Copy link
Copy Markdown
Collaborator

@kAIto47802 kAIto47802 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 your update! LGTM!

3. Choose the best point from the collected points,
4. Choose ``n_local_search`` points from the collected points using the roulette selection,
5. Perform a local search for each chosen point as an initial point, and
6. Return the point with the best acquisition function value as the next parameter.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for your update! I think there are a few slightly misleading parts, but considering the brevity, it would be acceptable, and the summary you wrote is really good!

@sawa3030
Copy link
Copy Markdown
Collaborator

Would it be helpful to explicitly clarify how convergence is determined in the local search, as raised in the first issue?

@nabenabe0928
Copy link
Copy Markdown
Contributor Author

Would it be helpful to explicitly clarify how convergence is determined in the local search, as raised in the first issue?

@sawa3030
I added a comment for this:)

Copy link
Copy Markdown
Member

@y0z y0z left a comment

Choose a reason for hiding this comment

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

I leave some comments.

Is the description of the multi-objective optimization in GPSampler outside the scope of this PR? E.g., updating the sampler functionality table.

Copy link
Copy Markdown
Collaborator

@sawa3030 sawa3030 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 update. LGTM

@codecov
Copy link
Copy Markdown

codecov bot commented May 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.37%. Comparing base (2832e92) to head (8006975).
⚠️ Report is 573 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6081      +/-   ##
==========================================
+ Coverage   88.34%   88.37%   +0.03%     
==========================================
  Files         207      207              
  Lines       13975    13995      +20     
==========================================
+ Hits        12346    12368      +22     
+ Misses       1629     1627       -2     

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

Copy link
Copy Markdown
Member

@y0z y0z left a comment

Choose a reason for hiding this comment

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

LGTM

@y0z y0z merged commit 219726d into optuna:master May 23, 2025
16 checks passed
@y0z y0z removed their assignment May 23, 2025
@not522 not522 added this to the v4.4.0 milestone May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

document Documentation related.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants