Handle NotPSDError and hitting maxiter in fit_gpytorch_model

[saitcakmak]

Motivation

The current behavior of fit_gpytorch_model is to error out when it receives a NotPSDError and to count hitting maxiter during optimization as an optimization failure.

Labeling maxiter as an optimization failure leads to retrying the model fit, and likely reaching maxiter in each try and labeling all tries as failures. This ends up using more effort than the user intended to (by passing in maxiter in the first place), while offering no real benefit. The proposed solution here is to simply count hitting maxiter as a successful model fit and to return the fit obtained with the given budget.

The NotPSDError during model fit is one of the most common reasons that leads to me having to restart an experiment. What I observed is that when the first try raises NotPSDError, a second try starting from initial parameters sampled from the priors will most of the time produce a successful model fit. So, this proposes to catch the NotPSDError and retry the model fit with new initial parameter values.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Unittests.

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/pytorch/botorch, and link to your PR here.)

cc @sdaulton

[Balandat]

Thanks, this makes a lot of sense to me! A few comments inline, but this looks good overall.

[codecov]

Codecov Report

Merging #1007 (9fd8ed4) into main (d0326cb) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main     #1007   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          110       110           
  Lines         8562      8570    +8     
=========================================
+ Hits          8562      8570    +8     

Impacted Files	Coverage Δ
botorch/fit.py	100.00% <100.00%> (ø)
botorch/utils/sampling.py	100.00% <0.00%> (ø)
botorch/acquisition/multi_objective/monte_carlo.py	100.00% <0.00%> (ø)

---

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 d0326cb...9fd8ed4. Read the comment docs.

[facebook-github-bot]

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

[Balandat]

Looks like we lost a tiny bit of test coverage since we don't actually fail fitting anymore other than with the NonPSDError. Would you mind adding a test for that (e.g.. fail with some other exception)?

[facebook-github-bot]

@saitcakmak has updated the pull request. You must reimport the pull request before landing.

[saitcakmak]

I didn't realize we needed settings.debug(True) for warnings to work. I spent some time wondering why my warning wasn't being caught...
Also, I can't find a way to use mock to produce warnings. I was looking for something like side_effect but it doesn't seem to support warnings. If you know of a way of wrapping a function with a warning, I'd love to learn!

[facebook-github-bot]

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