Skip to content
This repository has been archived by the owner on Nov 14, 2023. It is now read-only.

Fix BOHB, change n_iter -> n_trials, fix up early stopping #81

Merged
merged 29 commits into from
Aug 30, 2020

Conversation

richardliaw
Copy link
Collaborator

@richardliaw richardliaw commented Aug 29, 2020

You can see the diff here: inventormc/tune-sklearn-1@warm-start...richardliaw:bobh

Here are the main changes incorporated in the PR:

  1. We fix the BOHB example to actually leverage bohb properly.
  2. early stopping was not calling partial_fit, so that is also fixed.
  3. we rename n_iter to n_trials because n_iter is incredibly confusing (closes Rename n_iter => n_trials #80)

Comment on lines -13 to -25
space = {
"n_estimators": CS.UniformIntegerHyperparameter("n_estimators", 100, 200),
"min_weight_fraction_leaf": (0.0, 0.5),
"min_samples_leaf": CS.UniformIntegerHyperparameter(
"min_samples_leaf", 1, 5)
}

tune_search = TuneSearchCV(
RandomForestClassifier(),
space,
search_optimization="bohb",
n_iter=3,
max_iters=10)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Yard1 I ended up removing this example because:

  1. BOHB requires a form of iterative updating but
  2. random forests don't have an iterative update ability (from what I can read - please let me know if this is wrong!)

Happy to add this back, but would want to understand how to make this work practically.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with it being removed, thanks for working on this.

@Yard1
Copy link
Member

Yard1 commented Aug 29, 2020

@richardliaw So just to confirm, BOHB requires partial fit or warm start to work correctly, yes?

@richardliaw
Copy link
Collaborator Author

Yeah, same with other early stopping mechanisms.

return hasattr(self.main_estimator, "partial_fit")

@property
def main_estimator(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the catch on partial_fit not found for list objects!

@@ -143,8 +143,8 @@ class TuneSearchCV(TuneBaseSearchCV):
this parameter is ignored for ``"bohb"``, as it requires
``HyperBandForBOHB``.

n_iter (int): Number of parameter settings that are sampled.
n_iter trades off runtime vs quality of the solution.
n_trials (int): Number of parameter settings that are sampled.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would clear confusion with max_iters, but do you think it'd be helpful to specify that this is the equivalent of n_iter from scikit-learn? I remember we discussed naming max_iters before too so I'm wondering if it's a problem about this parameter's name or max_iters's.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm, good point. I think the reference to sklearn doesn't make sense since sklearn doesn't support "iterative models / leveraging partial fit". Since we do, it becomes a source of confusion.

@richardliaw richardliaw merged commit acb013c into ray-project:master Aug 30, 2020
@richardliaw richardliaw deleted the bobh branch August 30, 2020 00:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename n_iter => n_trials
4 participants