Several Local Bayesian Optimization Implementations #730
Conversation
dengdifan
changed the title
requirements.txt
Outdated
| @@ -9,3 +9,6 @@ pyrfr>=0.8.0 | |||
| lazy_import | |||
| dask | |||
| distributed | |||
| torch>=1.9.0 | |||
| gpytorch>=1.5.0 | |||
| pyro-ppl | |||
There was a problem hiding this comment.
Could you please also specify a minimal version that is required
|
python examples/fmin_rosenbrock_boing.py
|
| gpytorch.settings.debug.off() | ||
|
|
||
|
|
||
| class ExactGPModel(ExactGP): |
There was a problem hiding this comment.
Without any docstring it's difficult to see at a glance what this is used for.
| normalize_y: bool = True, | ||
| n_opt_restarts: int = 10, |
There was a problem hiding this comment.
Since they are not optional I suggest to move them before likelihood
| bounds_cont: np.ndarray | ||
| bounds of continuous hyperparameters | ||
| bounds_cat: typing.List[typing.List[typing.Tuple]], | ||
| bounds of categorical hyperparameters, need to be flattened, e.g. all the possible categorical needs to | ||
| be listed |
There was a problem hiding this comment.
I don't see whey those are necessary. The bounds argument should suffice to get these values already as the documentation states above. I.e. continuous hypers have the bounds (lower, upper) and categoricals have (n_cat, np.nan).
You can also have a look at the get_types function in util_funcs.py which takes care of getting the bounds.
Lines 12 to 74 in 55369ec
There was a problem hiding this comment.
Actually, I believe the resulting self.bound_cat and self.bound_cont are never used in the code anyways.
There was a problem hiding this comment.
That is only used in another GP model, I will fix that
| self.cont_dims = np.where(np.array(types) == 0)[0] | ||
|
|
||
| self.normalize_y = normalize_y | ||
| self.n_opt_restarts = n_opt_restarts |
There was a problem hiding this comment.
Please add a check somewhere that this parameter is a positive integer or otherwise handle illegal values.
|
|
||
| self.num_points = 0 | ||
|
|
||
| def _train(self, X: np.ndarray, y: np.ndarray, do_optimize: bool = True) -> 'GaussianProcess': |
There was a problem hiding this comment.
| def _train(self, X: np.ndarray, y: np.ndarray, do_optimize: bool = True) -> 'GaussianProcess': | |
| def _train(self, X: np.ndarray, y: np.ndarray, do_optimize: bool = True) -> GaussianProcessGPyTorch: |
since it returns self it should be typhinted with the correct class.
| self.bounds_cat = bounds_cat, | ||
| self.num_inducing_points = num_inducing_points | ||
|
|
||
| def update_attribute(self, **kwargs: typing.Any): |
There was a problem hiding this comment.
The type hint is wrong. kwargs (i.e. key-word arguments) will always be a dictionary.
The entries however can be of type typing.Any
| # A modification of from botorch.optim.utils._scipy_objective_and_grad, | ||
| # THe key difference is that we do an additional nature gradient update here |
There was a problem hiding this comment.
Please make this an actual docstring
| If both in and out are None: return an empty models | ||
| If only in_x and in_y are given: return a vanilla GP model | ||
| If in_x, in_y, out_x, out_y are given: return a partial sparse GP model. |
There was a problem hiding this comment.
What is the use case of an empty model?
I would rather suggest to have in_x and in_y as required arguments and only out_x and out_y as optional.
Further, to be consistent with naming in other classes it would be good if you could rename the "x" variables to "in_X" and "out_X" respectively.
There was a problem hiding this comment.
smac.epm.base_gp.BaseModel requires to run 'self._get_gp()' in 'init', I made all arguments optional to make it compatible with the super class
| except Exception as e: | ||
| if i == n_tries - 1: | ||
| raise e | ||
| continue |
There was a problem hiding this comment.
Please try to be more specific in what exceptions are okay to skip and add some documentation.
| def check_points_in_ss(X: np.ndarray, | ||
| cont_dims: np.ndarray, | ||
| cat_dims: np.ndarray, | ||
| bounds_cont: np.ndarray, | ||
| bounds_cat: typing.List[typing.List[typing.Tuple]], | ||
| ): |
There was a problem hiding this comment.
Might better live in the utils
Codecov Report
@@ Coverage Diff @@
## development #730 +/- ##
================================================
- Coverage 87.27% 20.14% -67.13%
================================================
Files 72 77 +5
Lines 6450 7600 +1150
================================================
- Hits 5629 1531 -4098
- Misses 821 6069 +5248
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
@dengdifan As promised I also provide some feedback to the abstract_subspace.
Except for a few minor things the abstract_subspace looks all good to me :)
| bounds_ss_cont: np.ndarray(D_cont, 2) | ||
| subspaces bounds of continuous hyperparameters, its length is the number of continuous hyperparameters | ||
| bounds_ss_cat: typing.List[typing.Tuple] | ||
| subspaces bounds of categorical hyperparameters, its length is the number of categorical hyperparameters |
There was a problem hiding this comment.
Could the bounds be used for this again or is this a set of different bounds?
| if rng is None: | ||
| self.rng = np.random.RandomState(1) | ||
| else: | ||
| self.rng = np.random.RandomState(rng.randint(0, 2 ** 20)) |
There was a problem hiding this comment.
I would have expected that if rng is not None it would already be a numpy RandomState.
At least according to the above docstring.
There was a problem hiding this comment.
To avoid using the same rng from outside as this could probably be a reference?
| if activate_dims is None: | ||
| activate_dims = np.arange(n_hypers) | ||
|
|
||
| if activate_dims is None: | ||
| activate_dims_cont = cont_dims | ||
| activate_dims_cat = cat_dims | ||
| self.activate_dims = np.arange(n_hypers) |
There was a problem hiding this comment.
The if statement twice checks the same condition.
|
|
||
| self.config_origin = "subspace" | ||
|
|
||
| def check_points_in_ss(self, |
There was a problem hiding this comment.
Wasn't this function also implemented in the PartialSparseGP?
|
I made some interface changes as the new implementation caused some confusions:
|
## Features * [BOinG](https://arxiv.org/abs/2111.05834): A two-stage Bayesian optimization approach to allow the optimizer to focus on the most promising regions. * [TurBO](https://arxiv.org/abs/1910.01739): Reimplementaion of TurBO-1 algorithm. * Updated pSMAC: Can pass arbitrary SMAC facades now. Added example and fixed tests. ## Improvements * Enabled caching for multi-objectives (#872). Costs are now normalized in `get_cost` or optionally in `average_cost`/`sum_cost`/`min_cost` to receive a single float value. Therefore, the cached cost values do not need to be updated everytime a new entry to the runhistory was added. ## Interface changes * We changed the location of Gaussian processes and random forests. They are in the folders `epm/gaussian_process` and `epm/random_forest` now. * Also, we restructured the optimizer folder and therefore the location of the acquisition functions and configuration chooser. * Multi-objective functions are located in the folder `multi_objective`. * pSMAC facade was moved to the facade directory. Co-authored-by: Difan Deng <deng@dengster.tnt.uni-hannover.de> Co-authored-by: Eddie Bergman <eddiebergmanhs@gmail.com> Co-authored-by: Carolin Benjamins <benjamins@tnt.uni-hannover.de> Co-authored-by: timruhkopf <timruhkopf@gmail.com>
Version 1.4.0 (automl#730, automl#855, automl#869, automl#872)
To run examples/fmin_rosenbrock_boing.py
You need to install pyrfr from this PR to get more interface (However, Ive not written test file for that PR)
automl/random_forest_run#64