GPJax icon indicating copy to clipboard operation
GPJax copied to clipboard

Feature: Adds probability of improvement as an acquisition function

Open miguelgondu opened this issue 1 year ago • 3 comments

Type of changes

  • [x] New feature
  • [x] Documentation / docstrings
  • [x] Tests

Checklist

  • [x] I've formatted the new code by running poetry run pre-commit run --all-files --show-diff-on-failure before committing.
  • [x] I've added tests for new code.
  • [x] I've added docstrings for the new code.

Checklist for this PR in particular

  • [x] Adding tests particular to the PI acquisition function.
  • [x] Decide on whether we want to call these UtilityFunctions or AcquisitionFunctions. (We decided to keep them as utility_functions)
  • [x] Decide on whether we want to write a whole new tutorial for these, or just expand the tutorial on BO. (We decided to add to an existing tutorial)
  • [x] Add docstrings.

Description

This PR expands the decision_making module by adding Probability of Improvement as a potential acquisition function. It refactors the tests that are joint to utility functions, and provides a tutorial for how to use Probability of Improvement in practice (pattern-matching from the current tutorial on Thompson Sampling).

Issue Number: N/A

miguelgondu avatar Jul 01 '24 12:07 miguelgondu

Hi @gpjax/developers,

I'm close to done with adding Probability of Improvement (PI) as an acquisition function. Before I finish this PR I would like to ask two questions:

  1. Should I create an entire new tutorial for BayesOpt beyond Thompson Sampling? Should I just update the current tutorial with a brief example of how to use PI? I vote for the latter. Currently I'm writing a new tutorial, but there's plenty of overlap with the current one on BO. It's worth noting that the current tutorial doesn't use the built-in functionalities for optimizing the acquisition function.
  2. I'm currently adding PI under the same folder as Thompson Sampling (i.e. utility_functions). I feel a better name for them might be acquisition_functions, but I'm happy either way. Let me know if I should be providing it under a different model than the abstract Utility Function.

miguelgondu avatar Jul 01 '24 12:07 miguelgondu

Hi @Thomas-Christie,

Thanks for the review! I've addressed all the comments. Could I ask you to take a second look? Let me know if anything else comes to mind.

Cheers.

miguelgondu avatar Jul 03 '24 08:07 miguelgondu

All green! 🎉

miguelgondu avatar Jul 05 '24 08:07 miguelgondu

Super work @miguelgondu ! Thanks for reviewing @Thomas-Christie

thomaspinder avatar Jul 06 '24 06:07 thomaspinder