Closed till-m closed 4 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 96.23%. Comparing base (
f2c720e
) to head (3d85734
). Report is 1 commits behind head on master.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Some more changes:
ConstantLiar
exploration_decay
KrigingBeliever
, where the GP was fitted on the wrong dataasync_optimization.py
script to showcase ConstantLiar
@fmfn @bwheelz36 @leandrobbraga I will wait with merging this until the docstrings are finished, but if you'd like to give some feedback already, please do.
@pfebrer this contains GPHedge, if you'd like to give some feedback on it.
@rizhiy I added the constant liar and the kriging believer for async optimization, as well as a script based on your code in examples/async_optimization_dummies.py
. Ideally this will eventually become a notebook. According to this, lying is the better strategy, but I wasn't sure whether it would make sense for constrained optimization, so I added the believer for that case.
Wow this is a huge amount of work. Impressive! Before we start review can you resolve the merge conflicts and make sure all the tests pass?
Before we start review can you resolve the merge conflicts and make sure all the tests pass?
Will do 👍 ideally I want to wait until the docs are finished to only deal with one conflict. After that I will mark the PR as ready to review :)
@till-m - will be a breaking redesign? Not necessarily against it, just want to be clear.
@till-m - will be a breaking redesign? Not necessarily against it, just want to be clear.
There will be some necessary breaking changes, primarily to workflows that change the acquisition function -- this is unavoidable, since the old UtilityFunction
object has been replaced. I also (intentionally) modified the way the acquisition function is changed -- it is now a property of the optimizer (i.e. not an argument to maximize/suggest). I understand that this can be annoying for some users, but I think in the long run it will be exactly those users that will profit from this change.
If someone just calls .maximize
nothing should change. Similarly, suggest
-probe
-register
w/o messing with the acquisition function will not change either.
@bwheelz36 I know this is a lot. I'm sorry about that 😬
Coverage is fixed now! :)
Agree on all other points. Thanks for the review! :)
Redesign the way acquisition functions work.
Content:
UtilityFunction
class and replaces it with a genericAcquisitionFunction
base classacq_max
, let each acquisition function handle the maximization itselfUpperConfidenceBound
,ProbabilityOfImprovement
andExpectedImprovement
as possible utility functionsGPHedge
to find which of (a combination of) UCB/PoI/EI are best suited for a problem #439KrigingBeliever
to allow for async optimization #347 #365~ConstantLiar
to allow for async optimization #347 #365. Strategies aremin
,max
,mean
andconstant
(latter chosen by supplying a number).UCB(kappa=2.576)
,PoI(xi=0.01)
,EI(xi=0.01)
.~ Sets default unconstrained acquisition function toUCB(kappa=2.576)
PoI(xi=0.01)
,EI(xi=0.01)
as standard acquisition function for constrained improvement~ Sets default constrained acquisition function toEI(xi=0.01)
suggest
,maximize
etc. don't take an acquisition function argument anymore.Why? The old
UtilityFunction
andacq_max
were the most inflexible part of this package and their design was somewhat convoluted (e.g. providing axi
parameter when using UCB). Now it should be much easier to add custom acquisition functions.