BayesianOptimization.maximize behaves unexpectedly when passing any of the deprecated parameters, in a non-backward-compatible way

[godot11]

In a (probably recent) major API change, passing the acq, kappa, kappa_decay, kappa_decay_delay, xi parameters to BayesianOptimization.maximize apparently became deprecated.

However, right now, BayesianOptimization.maximize accepts these parameters, and silently TOTALLY ignores them. Only a DeprecationWarning is emitted, (which is silent by default unless the function is in the top library), as seen in the relevant lines in bayesian_optimization:

    old_params_used = any([param is not None for param in [acq, kappa, kappa_decay, kappa_decay_delay, xi]])
        if old_params_used or gp_params:
            warnings.warn('\nPassing acquisition function parameters or gaussian process parameters to maximize'
                                     '\nis no longer supported, and will cause an error in future releases. Instead,'
                                     '\nplease use the "set_gp_params" method to set the gp params, and pass an instance'
                                     '\n of bayes_opt.util.UtilityFunction using the acquisition_function argument\n',
                          DeprecationWarning, stacklevel=2)

        if acquisition_function is None:
            util = UtilityFunction(kind='ucb',
                                   kappa=2.576,
                                   xi=0.0,
                                   kappa_decay=1,
                                   kappa_decay_delay=0)
        else:
            util = acquisition_function

This behavior does not make sense. It is absolutely NOT backwards compatible, and worse, breaks existing code in a very confusing, unexpected, and hard-to-debug way. Backwards compatibility doesn't mean that the change won't cause syntax error in existing code, it means that existing code will function as before.

In this case, either the values should be passed to the UtilityFunction, or a ValueError should be emitted.

In fact, this way of implementation caused our team to lose a week worth of effort, as we wanted to compare the three implemented utility function, and the effect of different kappa and kappa-decay values on 'ucb'. We were puzzled why we don't see any variation like previously using the same code. As we call BayesianOptimization.maximize from within a homebrew library, the warning was never printed, everything seemed to be fine, and the code just silently replaced all of our parameters with the default values.

Even if the warning was emitted, however, it doesn't tell that the values are already being ignored. It's just a warning that this API is deprecated and will be turned into an error.

Is there any reason why the values of acq, kappa, kappa_decay, kappa_decay_delay, xi are not passed to the created UtilityFunction, and are replaced with some default parameters, silently ignoring the explicit request of the coder?

[bwheelz36]

Hi @godot11
This change was made in #382; you can also read the rationale there. Basically it makes no sense to allow one to overwrite parameters that are previously (and appropriately) set with set_gp_params or when the Utility Function is created.

However, in hindsight the way this was implemented was clearly a mistake; it would have been more appropriate to raise an error. A learning experience for me. I will update this in a new PR.

[bwheelz36]

@godot11 please review above PR and let me know what you think

[bwheelz36]

Thank you @till-m for the review. I have updated the code and released a new version.
@godot11 I appreciate that this was a frustrating issue for you - but in the future can I suggest you put at least as much effort into fixing problems as you do documenting them - everyone here is working voluntarily.

[godot11]

Thanks for the fix @bwheelz36 , I appreciate it. I wasn't available for work for the past two weeks, I'm not sure why this is a problem.