BayesianOptimization icon indicating copy to clipboard operation
BayesianOptimization copied to clipboard

Constrained optimization

Open till-m opened this issue 1 year ago • 3 comments

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 avatar Aug 07 '22 12:08 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.

till-m avatar Aug 07 '22 12:08 till-m

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%) :arrow_up:

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

codecov-commenter avatar Aug 09 '22 22:08 codecov-commenter

@bwheelz36 Could you run the workflow?

till-m avatar Aug 10 '22 16:08 till-m

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?

bwheelz36 avatar Aug 14 '22 23:08 bwheelz36

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 avatar Aug 18 '22 18:08 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 🙃.

till-m avatar Aug 18 '22 19:08 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.

till-m avatar Aug 25 '22 11:08 till-m

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 :-)

bwheelz36 avatar Aug 25 '22 22:08 bwheelz36

That should be all, I hope.

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

till-m avatar Aug 27 '22 10:08 till-m

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

bwheelz36 avatar Aug 29 '22 00:08 bwheelz36

thanks for this work @till-m!

bwheelz36 avatar Aug 30 '22 05:08 bwheelz36