Skip to content

Refactoring of successive halving.#808

Merged
toshihikoyanase merged 3 commits intooptuna:masterfrom
hvy:refactor-successive-halving
Dec 20, 2019
Merged

Refactoring of successive halving.#808
toshihikoyanase merged 3 commits intooptuna:masterfrom
hvy:refactor-successive-halving

Conversation

@hvy
Copy link
Copy Markdown
Member

@hvy hvy commented Dec 18, 2019

This PR can probably wait until #785 is merged. I was mostly looking through the code and made these changes for my own readability.

A minor refactoring of the successive halving pruner that reduces calls to _completed_rung_key, changes some variable named and introduces indirections.

If you find the indirections to increase the complexity, please feel free to close this PR.

@hvy hvy added the code-fix Change that does not change the behavior, such as code refactoring. label Dec 18, 2019
@crcrpar crcrpar mentioned this pull request Dec 18, 2019
@sile sile self-requested a review December 19, 2019 08:48
Copy link
Copy Markdown
Member

@sile sile 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 PR.

I executed a benchmarking to confirm that this PR does not cause unintended behavioral changes (see below).

// Install kurobako (benchmark tool).
$ curl -L https://github.com/sile/kurobako/releases/download/0.1.3/kurobako-0.1.3.linux-amd64 -o kurobako
$ chmod +x kurobako && sudo mv kurobako /usr/local/bin/

// Download benchmark data file.
$ curl -OL http://ml4aad.org/wp-content/uploads/2019/01/fcnet_tabular_benchmarks.tar.gz
$ tar xf fcnet_tabular_benchmarks.tar.gz && cd fcnet_tabular_benchmarks/

// Create a problem recipe.
$ kurobako problem hpobench fcnet_parkinsons_telemonitoring_data.hdf5 > problem.json

// Benchmark `SuccessiveHalvingPruner` by using `optuna/optuna:0.19.0`.
$ pip uninstall -y optuna; pip install -U optuna==0.19.0
$ kurobako solver --name 'SuccessiveHalving@0.19.0' optuna --pruner asha > solver-0.19.0.json
$ kurobako studies --solvers $(cat solver-0.19.0.json) --problems $(cat problem.json) --repeats 1 --seed 1 | kurobako run > result-0.19.0.json

// Benchmark `SuccessiveHalvingPruner` by using `hvy/optuna:refactor-successive-halving`.
$ pip install git+https://github.com/hvy/optuna@refactor-successive-halving
$ kurobako solver --name 'SuccessiveHalving@refactor-successive-halving' optuna --pruner asha > solver-refactor-successive-halving.json
$ kurobako studies --solvers $(cat solver-refactor-successive-halving.json) --problems $(cat problem.json) --repeats 1 --seed 1 | kurobako run > result-refactor-successive-halving.json

// Compare the optimization (benchmarking) results.
$ jq '.trials[].evaluations[].values' result-0.19.0.json | head
[
  0.4457722008228302
]
[
  0.43835049867630005
]
[
  0.4163028597831726
]

$ jq '.trials[].evaluations[].values' result-0.19.0.json | wc -l
5997

$ jq '.trials[].evaluations[].values' result-0.19.0.json | md5sum
63f3ce3900f7b230e51bc5b954b2fa45  -

$ jq '.trials[].evaluations[].values' result-refactor-successive-halving.json | md5sum
63f3ce3900f7b230e51bc5b954b2fa45  -

As you can see the outputs of the last two commands, the SuccessiveHalvingPruner in this PR branch produced the exact same optimization result with the v0.19.0 one (i.e., there is no performance degradation).

@sile sile added this to the v0.20.0 milestone Dec 19, 2019
@toshihikoyanase toshihikoyanase self-requested a review December 20, 2019 01:26
@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 20, 2019

Codecov Report

Merging #808 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #808      +/-   ##
==========================================
+ Coverage   90.15%   90.15%   +<.01%     
==========================================
  Files         106      106              
  Lines        8768     8769       +1     
==========================================
+ Hits         7905     7906       +1     
  Misses        863      863
Impacted Files Coverage Δ
optuna/pruners/successive_halving.py 94.82% <100%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update baa2482...c4c5fe7. Read the comment docs.

Copy link
Copy Markdown
Member

@toshihikoyanase toshihikoyanase left a comment

Choose a reason for hiding this comment

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

LGTM.

@toshihikoyanase toshihikoyanase merged commit dd364ec into optuna:master Dec 20, 2019
@hvy hvy deleted the refactor-successive-halving branch December 20, 2019 05:20
@hvy hvy changed the title Refactoring of successive halving Refactoring of successive halving. Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-fix Change that does not change the behavior, such as code refactoring.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants