Advanced constrained optimization

[till-m]

Model constraints using Gaussian Process Regression and incorporate them into the selection of new points to sample.

Cf. #326 #304 #266 #276 #190 (sort of) #189.

For examples on how to use, see examples/constraints.ipynb.

I think some more documentation is necessary, but I would like to get feedback on the current state before I do that.

[till-m]

Please consider this a draft pull request as of now, I marked as ready to review to trigger the CI but apparently I need approval for that first.

[codecov-commenter]

Codecov Report

Merging #344 (a46c335) into master (cc05408) will decrease coverage by 0.01%.
The diff coverage is 99.24%.

@@            Coverage Diff             @@
##           master     #344      +/-   ##
==========================================
- Coverage   99.26%   99.25%   -0.02%     
==========================================
  Files           7        8       +1     
  Lines         409      535     +126     
  Branches       58       81      +23     
==========================================
+ Hits          406      531     +125     
- Misses          1        2       +1     
  Partials        2        2              

Impacted Files	Coverage Δ
bayes_opt/constraint.py	98.38% <98.38%> (ø)
bayes_opt/__init__.py	100.00% <100.00%> (ø)
bayes_opt/bayesian_optimization.py	100.00% <100.00%> (ø)
bayes_opt/observer.py	100.00% <100.00%> (ø)
bayes_opt/target_space.py	100.00% <100.00%> (ø)
bayes_opt/util.py	97.77% <100.00%> (+0.13%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[till-m]

@bwheelz36 Could you run the workflow?

[bwheelz36]

Hey, thanks for this, it looks awesome.

So, am I correct in thinking that this is based off this work by gardner?

In that case, this handles situations where checking feasibility is just as expensive as assessing the objective function? It appears that the objective function is still assessed for unfeasible cases, but that the likelihood of probing these regions should decrease as the GP models get better?

What about the more common scenario where in/feasibility is known in advance? it seems like in that case, this is quite a complex and expensive approach to deal with constraints? Do you think there's an elegant way to handle both scenarios - the case where the feasible regions are known in advance and the case where they are only known retrospectively?

Sorry if these are dumb questions, I freely admit I don't entirely have my head around the complexity of constrained bayesian optimization!
Also, that plotting routine is a very nice visualization.

[till-m]

So, am I correct in thinking that this is based off this work by gardner?

Yes! This was the approach discussed in #326.

In that case, this handles situations where checking feasibility is just as expensive as assessing the objective function? It appears that the objective function is still assessed for unfeasible cases, but that the likelihood of probing these regions should decrease as the GP models get better?

Yes, or at least very expensive.

What about the more common scenario where in/feasibility is known in advance? it seems like in that case, this is quite a complex and expensive approach to deal with constraints?

I think you are right. The reason why I went for this approach was 1) the robustness/versatility and 2) because the paper seemed interesting. Nonetheless, it is probably unnecessarily complex for many usecases.

Do you think there's an elegant way to handle both scenarios - the case where the feasible regions are known in advance and the case where they are only known retrospectively?

It's probably feasible to add some kind of mode="simple" or mode="gardner" to the ConstraintModel. How would you propose handling easy constraints in practice?

Also, that plotting routine is a very nice visualization.

Thanks, it took longer to write that than I'd like to admit ;)

I will update the internal documentation and add more comments to the notebook as well.

[till-m]

I notice a lot of these properties are prepended with a _ and equipped with a separate method essentially acting as a getter, presumably to avoid users setting these properties. Let me know which properties you want me to handle this way.

[bwheelz36]

hey, I might consider putting self.constraint as self._constraint - which would be consistent with how _gp is currently handled.

[bwheelz36]

hey @till-m - can I suggest the following edits on the documentation:

• change file name to advanced_constraints
• add in a introduction similar to the one below:

"In some situations, certain regions of the solution domain mat be infeasible or not allowed. In addition, you may not know where these regions are until you have evaluated the objective function at these points. In other words, checking for feasibility is as expensive, or close to as expensive as evaluating the objective function. This notebook demonstrates how you can handle these situations by also modelling the constraints as a Gaussian process. This approach is based on this paer by gardner et. al..
Note that this is not an appropriate approach to use if you already know what the constrained regions are prior to evaluating them. In this case, at the time of writing the best approach is to return a low number to the optimizer when it tries to evaluate these regions"

Regarding your question above - how would we handle the case where the constraints are known in advance - good question! I have done this the way I suggested above. It worked pretty well for me. However, I guess the problem arises where the best value is right near the feasible/non-feasible split - depending on how well the GP is working, these boundary regions may not be well modeled. Therefore a more elegant solution would be to properly constrain the space in the first place - but I don't really know how to do that properly and I have to think about it some more!

In the meantime - following the changes above this PR looks pretty ready to me, do you agree?

[till-m]

Hi @bwheelz36,

I think I misunderstood -- you intended for me to rename the notebook, not the .py file, right?

If yes, I would suggest a slightly different approach: Retain the name constraints.ipynb. I will add documentation around the advanced constraints with an introduction like you suggested and a few more hints etc. in markdown cells. Barring other holdups, we go ahead with the merge of this PR. Afterwards you illustrate the simple approach in the same notebook (above the advanced section). I think this is the cleanest solution (or a good stop-gap if we plan on adding an internal way of handling simple constraints). The only concern I'd have is that the notebook could end up somewhat lengthy, but I think that's unlikely.

Let me know what you think. I think your approach should definitely be illustrated somewhere if we decide to mention it in the notebook.

[till-m]

I've implemented the changes as suggested above, but feel free to let me know if you'd prefer a different approach. If you can, please proofread the documentation and let me know if there is something that should be modified -- be it content-wise or with respect to grammar and spelling. I am somewhat tired so I likely messed up something somewhere 🙃.

[bwheelz36]

This looks excellent, sorry I was slow to respond!
Main comment: should ConstraintModel be constructed by the user as it is now, ot should it be constructed when an optimizer is instantiated?
I think at the moment, we can have two different random_states set - it is probably cleaner if the constraint model gets the random state from the optimizer. In that case we would pass a dictionary of constraints and construct the model inside BayesianOptimization.

Another thought: we could have the user define their constraints using scipys nonlinear constraints. This would have the advantage that it's easier to switch into different algorithms, and thinking ahead, might make it easier for us to handle the simpler case of known constraints because the constraint can be directly passed to the acquisition function optimizer.

For the record I think this is excellent already and will merge as is, just let me know :-)

[till-m]

I think at the moment, we can have two different random_states set - it is probably cleaner if the constraint model gets the random state from the optimizer.

Agreed.

In that case we would pass a dictionary of constraints and construct the model inside BayesianOptimization.

Sounds good to me. Your approach has the additional advantage of avoiding side effects that might occur with modifying the original ConstraintModel instance, which I like.
The only thing I'm unsure about with this approach is how to expose the documentation to the user. Right now, the information is contained in the ConstraintModel docstring. Do you see any clever solution here?

(Another option would be to allow for both, a dict and an instance of ConstraintModel, though I am not sure if that doesn't just produce more clutter -- not super keen on this I think?)

Another thought: we could have the user define their constraints using scipys nonlinear constraints. This would have the advantage that it's easier to switch into different algorithms, and thinking ahead, might make it easier for us to handle the simpler case of known constraints because the constraint can be directly passed to the acquisition function optimizer.

I like this! I wasn't aware that such a class existed within sklearn. I think especially looking towards the future, this seems like an excellent idea.
Two things to note: Currently the code isn't designed to handle two-sided constraints (i.e. with lower and upper bounds) since that wasn't a part of the Gardner paper. I haven't thought about it too much, but I don't see an issue with implementing this -- it should just be another call to .cdf in .predict around here and calculating the difference between lower and upper. Finally, NonlinearConstraint allows equality constraints -- I don't see any way of making this work, but I don't think that's a problem. I can just check whether the user defined an equality and raise a ValueError or RuntimeError.

For the record I think this is excellent already and will merge as is, just let me know :-)

Thanks, I appreciate it! I should be able to get most of this done over the week-end, I think.

[bwheelz36]

The only thing I'm unsure about with this approach is how to expose the documentation to the user. Right now, the information is contained in the ConstraintModel docstring. Do you see any clever solution here?

In general the documentation for this code is all in the example notebooks and relies on the user to go look at the code to figure out other options. While this might not be ideal, it does mean that what you have already done is consistent with the other features of the code.
Long term, and if we were to ever get control of the admin settings, we might be able to set up some sphinx html that would integrate the docstrings into a website....

(Another option would be to allow for both, a dict and an instance of ConstraintModel, though I am not sure if that doesn't just produce more clutter -- not super keen on this I think?)

Yeah I agree, keep it simple :-)

[till-m]

That should be all, I hope.

I think #351 should be merged first since there might be conflicts.

[bwheelz36]

thanks @till-m - that one is merged now!

[bwheelz36]

thanks for this work @till-m!