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

Add tutorial for TuRBO-1 #598

Closed
wants to merge 3 commits into from
Closed

Add tutorial for TuRBO-1 #598

wants to merge 3 commits into from

Conversation

dme65
Copy link
Contributor

@dme65 dme65 commented Nov 17, 2020

This adds a tutorial for TuRBO-1.

Motivation

To allow people to use, improve upon, and experiment with TuRBO in BoTorch.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

This only adds a notebook, so no additional tests should be needed.

Related PRs

N/A

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Nov 17, 2020
@Balandat
Copy link
Contributor

Thanks for putting this up, overall this lgtm.

A few comments:

Test failure is unrelated - should disappear afterrebase.

Copy link
Contributor

@eytan eytan left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me, added two brief comments.

"outputs": [],
"source": [
"def create_initial_state(dim, batch_size):\n",
" return {\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest using a dataclass or namedtuple here.

"\n",
"TuRBO-1 is a local optimizer that can be used for a fixed evaluation budget in a multi-start fashion. Once TuRBO converges, `state[\"restart_triggered\"]` will be set to true and the run should be aborted. If you want to run more evaluations with TuRBO, you simply generate a new set of initial points and then keep generating batches until convergence or when the evaluation budget has been exceeded. It's important to note that evaluations from previous instances are discarded when TuRBO restarts.\n",
"\n",
"NOTE: We use a `FixedNoiseGP` in this case as the problem is noise-free."
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if we want to encourage users to input noisy observations when no observation noise is present. Inferring the noise can help with numerical stability and help protect against model misspecification (see e.g., https://arxiv.org/pdf/1007.4580.pdf)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The thought here is that the FixedNoiseGP with a little bit of noise acts as regularization, but it makes sense to learn. I can switch over to the SingleTaskGP and add a constraint + prior to keep the learned noise from being large since we know the problem is noise-free.

@codecov
Copy link

codecov bot commented Dec 1, 2020

Codecov Report

Merging #598 (ac49ef1) into master (0f9d3c7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #598   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           91        91           
  Lines         5957      5957           
=========================================
  Hits          5957      5957           

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 0f9d3c7...ac49ef1. Read the comment docs.

@dme65
Copy link
Contributor Author

dme65 commented Dec 1, 2020

Thanks for fixing the failing tests on master, @Balandat! I've made some changes, what do you think?

Copy link
Contributor

@Balandat Balandat left a comment

Choose a reason for hiding this comment

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

lgtm, thanks! Feel free to import and land.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@dme65 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@dme65 merged this pull request in c0ab26e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants