Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kappa decay #221

Closed
wants to merge 23 commits into from
Closed

Conversation

ben-arnao
Copy link
Contributor

Add a tunable parameter kappa decay that will allow you to make the optimization more exploitative over time. From a few tests I've run i get faster convergence to better maxima.

Add a tunable parameter kappa decay that will allow you to make the optimization more exploitative over time. From a few tests I've run i get faster convergence to better maxima.
@fmfn
Copy link
Member

fmfn commented Apr 27, 2020

Thanks for the PR. I really like the idea of adding this. However I'm not a fan of recreating the UtilityFunction object like that, can you add a update_kappa (or similarly named) method to UtilityFunction and use that inside the maximize loop instead? That way this feature is also accessible outside the maximize function too.

@@ -162,9 +163,16 @@ def maximize(self,
self._prime_queue(init_points)
self.set_gp_params(**gp_params)

util = UtilityFunction(kind=acq, kappa=kappa, xi=xi)
def decay_kappa(kappa, iters, decay):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we turn this into a UtilityFunction method?

iteration = 0
while not self._queue.empty or iteration < n_iter:
bo_iters = max(0, len(self.res) - init_points)
decayed_kappa = decay_kappa(init_kappa, bo_iters, kappa_decay)
util = UtilityFunction(kind=acq, kappa=decayed_kappa, xi=xi)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the method to update the value here as opposed to creating a new object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put the decay in UtilityFunction and also added an optional "delay" parameter to give users the option to delay kappa decay by X BO iters before kappa starts to decay.

I think this will be useful for ensuring a fixed amount of explorative BO in the beginning steps before decaying kappa to be more exploitative. As with original kappa decay it is optional and by default turned off so most users will not even know this is an option. Doesn't change the existing API either.

Lmk what you think.

also add an additional parameter if the user wants to delay the kappa decay for a certain amount of BO iterations
@codecov-io
Copy link

codecov-io commented Apr 27, 2020

Codecov Report

Merging #221 into master will decrease coverage by 0.91%.
The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #221      +/-   ##
==========================================
- Coverage   99.68%   98.77%   -0.92%     
==========================================
  Files           6        6              
  Lines         322      327       +5     
  Branches       32       33       +1     
==========================================
+ Hits          321      323       +2     
- Misses          0        2       +2     
- Partials        1        2       +1     
Impacted Files Coverage Δ
bayes_opt/bayesian_optimization.py 97.97% <0.00%> (-2.03%) ⬇️
bayes_opt/util.py 98.14% <50.00%> (-0.90%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 600d0c7...cfd10b3. Read the comment docs.

@@ -99,6 +99,9 @@ def utility(self, x, gp, y_max):
return self._ei(x, gp, y_max, self.xi)
if self.kind == 'poi':
return self._poi(x, gp, y_max, self.xi)

def decay_kappa(self, decay_factor):
Copy link
Member

@fmfn fmfn Apr 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant passing the decay_factor (and _delay too) to the UtilityFunction constructor, and then calling util.update_params() (or similar name) in the loop. All this logic if kappa_decay != 1 and max(0, len(self.res) - init_points) - kappa_decay_delay > 0: could reside inside the UtilityFunction object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah i see. I will make the changes. For delay to be used however we will need to feed in the # of BO iters to the update function. Lmk how it looks now.

Copy link
Member

@fmfn fmfn Apr 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UtilityFunction could keep an internal counter to keep track of delay which is updated every time the method for kappa decay is invoked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok done

@@ -165,6 +167,10 @@ def maximize(self,
util = UtilityFunction(kind=acq, kappa=kappa, xi=xi)
iteration = 0
while not self._queue.empty or iteration < n_iter:
# decay kappa by decay factor if BO iters exceeds kappa_decay_delay
if kappa_decay != 1 and max(0, len(self.res) - init_points) - kappa_decay_delay > 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rather not introduce this if statement here. It's at a different abstraction level than the surrounding code and it also only applies to one kind of utility function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this still something you wanted to move forward with?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, absolutely. I've been a bit busy lately with work, so I'm slow to make comments and stuff. But we'll get this merged this week =)

@fmfn
Copy link
Member

fmfn commented May 5, 2020

The implementation is great! I'm ready to approve, but I have one small ask first. Would you mind rebasing and squashing all commits in a single one?

Add a tunable parameter kappa decay that will allow you to make the optimization more exploitative over time. From a few tests I've run i get faster convergence to better maxima.

move decay to a UtilityFunction method

also add an additional parameter if the user wants to delay the kappa decay for a certain amount of BO iterations

add decay to UtilityFunction

Update bayesian_optimization.py

Update bayesian_optimization.py

Update bayesian_optimization.py

Update bayesian_optimization.py

Update util.py

Update util.py

Update util.py

Update util.py

Update bayesian_optimization.py

Update util.py

Update bayesian_optimization.py

Update bayesian_optimization.py
parent 19a2314
author ben-arnao <arnaoben@gmail.com> 1588781383 -0400
committer ben-arnao <arnaoben@gmail.com> 1588788584 -0400

Kappa decay

Add a tunable parameter kappa decay that will allow you to make the optimization more exploitative over time. From a few tests I've run i get faster convergence to better maxima.

move decay to a UtilityFunction method

also add an additional parameter if the user wants to delay the kappa decay for a certain amount of BO iterations

add decay to UtilityFunction

Update bayesian_optimization.py

Update bayesian_optimization.py
@ben-arnao ben-arnao mentioned this pull request May 6, 2020
@ben-arnao ben-arnao closed this May 6, 2020
@bwheelz36
Copy link
Collaborator

Hey guys, this is a really cool feature, but it took me a while to realise it was here - unless I missed it I don't think this is covered in the examples?
If it's helpful I could do a PR with an update to the advanced tour that demonstrated this feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants