Skip to content

Get system_attrs lazily for samplers and pruners#4603

Closed
not522 wants to merge 2 commits intooptuna:masterfrom
not522:lazy-trial-system-attrs
Closed

Get system_attrs lazily for samplers and pruners#4603
not522 wants to merge 2 commits intooptuna:masterfrom
not522:lazy-trial-system-attrs

Conversation

@not522
Copy link
Copy Markdown
Member

@not522 not522 commented Apr 12, 2023

Motivation

This is part of the work to deprecate _CachedStorage.

When directly using RDBStorage, _get_latest_trial is a bottleneck because it reads system_attrs from the remote storage.

Description of the changes

Getting system_attrs lazily. This change eliminates access to remote storage for system_attrs by samplers and pruners except GridSampler, SuccessiveHalvingPruner, and HyperbandPruner.

@not522 not522 added the enhancement Change that does not break compatibility and not affect public interfaces, but improves performance. label Apr 12, 2023
@github-actions github-actions bot added the optuna.trial Related to the `optuna.trial` submodule. This is automatically labeled by github-actions. label Apr 12, 2023
@not522 not522 marked this pull request as draft April 12, 2023 08:54
@not522 not522 marked this pull request as ready for review April 17, 2023 04:32
@c-bata c-bata self-assigned this Apr 17, 2023
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #4603 (7ff4b7d) into master (1599d23) will increase coverage by 0.17%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #4603      +/-   ##
==========================================
+ Coverage   90.79%   90.96%   +0.17%     
==========================================
  Files         187      183       -4     
  Lines       13932    13925       -7     
==========================================
+ Hits        12649    12667      +18     
+ Misses       1283     1258      -25     
Impacted Files Coverage Δ
optuna/trial/_trial.py 95.67% <100.00%> (+0.29%) ⬆️

... and 23 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@not522 not522 force-pushed the lazy-trial-system-attrs branch from 7ff4b7d to 1c97089 Compare April 17, 2023 05:23
Copy link
Copy Markdown
Member

@c-bata c-bata left a comment

Choose a reason for hiding this comment

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

My initial impression is that changes look almost perfect to me. Let me check the performance of _CachedStorage with this change.

@toshihikoyanase
Copy link
Copy Markdown
Member

@gen740 Could you review this PR, please?

@not522
Copy link
Copy Markdown
Member Author

not522 commented Apr 24, 2023

This PR was taken over by #4631.

@not522 not522 closed this Apr 24, 2023
@not522 not522 deleted the lazy-trial-system-attrs branch April 24, 2023 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Change that does not break compatibility and not affect public interfaces, but improves performance. optuna.trial Related to the `optuna.trial` submodule. This is automatically labeled by github-actions.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants