Add a pruner based on ASHA #236
Conversation
toshihikoyanase
added
the
feature
|
@g-votte @toshihikoyanase @iwiwi I updated documentation of |
|
@g-votte @toshihikoyanase |
|
@g-votte @toshihikoyanase |
| assert 'completed_rung_1' not in trial.system_attrs | ||
|
|
||
|
|
||
| def test_successive_halving_pruner_reduction_factor_parameter(): |
There was a problem hiding this comment.
Please add a type hint.
| assert 'completed_rung_2' not in trial.system_attrs | ||
|
|
||
|
|
||
| def test_successive_halving_pruner_min_early_stopping_rate_parameter(): |
|
|
||
| study = optuna.study.create_study() | ||
|
|
||
| # min_resource=1: The rung 0 ends at step 1. |
There was a problem hiding this comment.
I think we can add an edge case where min_resource < 1 to confirm SuccessiveHalvingPruner.__init__() raises an appropriate exception. Also, please take care of the cases where reduction_factor < 2 and min_early_stopping_rate < 0 in test_successive_halving_pruner_reduction_factor_parameter and test_successive_halving_pruner_min_early_stopping_rate_parameter, respectively.
| min_resource=1, reduction_factor=2, min_early_stopping_rate=0) | ||
|
|
||
| # A pruner is not activated at a first trial. | ||
| assert not pruner.prune(study.storage, study.study_id, trial.trial_id, step=1) |
There was a problem hiding this comment.
I could not find the case where the target value of the pruner is NaN. Please add a test function like test_successive_halving_pruner_with_nan to confirm trials with NaN values are pruned without any exceptions.
| assert 'completed_rung_3' not in trial.system_attrs | ||
|
|
||
|
|
||
| def test_successive_halving_pruner_first_trial_always_wins(): |
There was a problem hiding this comment.
Because of "wins", I think this method name is a bit misleading. As there's no competition among trials in this test scenario, how about test_successive_halving_pruner_first_trial_is_not_pruned?
| assert pruner.prune(study.storage, study.study_id, trial.trial_id, step=1) | ||
|
|
||
|
|
||
| def test_successive_halving_pruner_up_to_third_rung(): |
There was a problem hiding this comment.
With this method name, I feel as if third were a boundary value. How about renaming this method to test_successive_halving_pruner_rung_check?
|
@toshihikoyanase @g-votte I updated |
This PR contains the following changes (mainly):
optuna/pruners/directory and splitpruners.pyintopruners/base.pyandpruners/median.pySuccessiveHalvingPrunerclassSuccessiveHalvingPruner