Botorch closures #1439

j-wilson · 2022-10-03T22:32:52Z

Summary:
Changelog:

Enable user-defined loss closures.
fit_gptorch_torch rewrite
Add fit_gyptorch_mll dispatch for ApproximateGPs

Differential Revision: D39101211

facebook-github-bot · 2022-10-03T22:33:43Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-10-03T23:48:15Z

This pull request was exported from Phabricator. Differential Revision: D39101211

Balandat · 2022-10-04T03:19:40Z

botorch/fit.py

+        data_loader: Convience keyword for passing in a DataLoader instance or dict of
+            keyword arguments passed to `get_data_loader` to obtain one. May only be


It feels a bit weird that data_loader can either be an instance or a set of kwargs passed to a factory function.

botorch/fit.py

Balandat · 2022-10-04T03:31:47Z

botorch/optim/closures.py

+
+dispatcher = Dispatcher("get_loss_closure")
+NoneType = type(None)
+TLossClosure = Callable[[], Tensor]


Is this the right type if you're forwarding kwargs to the MLL?

Not really, no. If we end up allowing kwargs to be passed to loss closures (debatable since torch.jit.script does not support this), we'll make TLossClosure a typing.Protocol.

botorch/optim/closures.py

Balandat · 2022-10-04T03:38:35Z

botorch/optim/fit.py

+    if isinstance(torch_optimizer, (type, partial)):
+        torch_optimizer = torch_optimizer(params=list(param_dict.values()))


I am not the biggest fan of the pattern of allowing this to be either an instance or a factory function. Not sure if there is an elegant alternative though.

Would factory only be better or worse? Note that instances can still be passed as lambda **_: instance, but the typing is now more explicit.

My $0.02: being able to pass an Instance is nicer than the factory, if only because then I don't necessarily need all the parameters I want to optimize to live somewhere on mll. There's even a reasonable argument to be made for accepting lists of Optimizers and stepping all of them, to support e.g. NGD or different Optimizers / learning rates for the GP vs a DNN

being able to pass an Instance is nicer than the factory, if only because then I don't necessarily need all the parameters I want to optimize to live somewhere on mll.

So, here's what we can do. We can split up fit_gpytorch_torch into a pair of methods: i) a generic torch-based gradient descent method and ii) a wrapper that provides mll-based model fitting conveniences. This seems to address your first point.

There's even a reasonable argument to be made for accepting lists of Optimizers and stepping all of them, to support e.g. NGD or different Optimizers / learning rates for the GP vs a DNN

Interesting idea. I like that this eliminates the need for step_limit. If you wanted to go further, you could pass in a list of OptimizationStep objects with slots for closure, optimizer, etc . Unclear to me whether we run into any scheduling issues here.

botorch/optim/fit.py

Balandat · 2022-10-04T03:44:17Z

botorch/optim/fit.py

+            torch_scheduler.step()
+
+    loss = None if loss is None else loss.detach().cpu().item()
+    return mll, {"fopt": loss, "wall_time": monotonic() - start_time}


should we include iterations here for backwards compatibility? I guess this is sufficiently deep in the stack that we don't need to worry too much about this

Callbacks Are All You Need. Wilson et al., 2022.

Jokes aside: Is it just a backward compatibility thing or are you thinking that some conveniences would be useful here?

botorch/optim/numpy_converter.py

Balandat · 2022-10-04T03:48:15Z

botorch/optim/numpy_converter.py

+    torch_closure: Optional[TLossClosure] = None,
+    parameter_setter: Callable[[np.ndarray], Any] = set_params_with_array,
+) -> Callable[[], Tuple[np.ndarray, np.ndarray]]:
+    if torch_closure is None:


add a docstring here?

botorch/optim/numpy_converter.py

botorch/optim/fit.py

jacobrgardner · 2022-10-04T04:35:50Z

This looks like a pretty reasonable solution to me! I actually like the ability to roll my own loss closures and optimizers here, and think it's worth the extra engineering. @nataliemaus has code that sometimes wants to train a GP end to end with a VAE included in the ELBO, and sometimes only wants to train the GP, so this should let us switch even more code into using BoTorch routines.

facebook-github-bot · 2022-10-04T18:34:41Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-10-05T01:08:15Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-10-10T00:06:32Z

This pull request was exported from Phabricator. Differential Revision: D39101211

codecov · 2022-10-10T00:10:39Z

Codecov Report

Merging #1439 (4f6c4ef) into main (7613cd2) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              main     #1439    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          134       143     +9     
  Lines        12402     12755   +353     
==========================================
+ Hits         12402     12755   +353

Impacted Files	Coverage Δ
botorch/models/pairwise_gp.py	`100.00% <ø> (ø)`
botorch/fit.py	`100.00% <100.00%> (ø)`
botorch/optim/__init__.py	`100.00% <100.00%> (ø)`
botorch/optim/closures/__init__.py	`100.00% <100.00%> (ø)`
botorch/optim/closures/core.py	`100.00% <100.00%> (ø)`
botorch/optim/closures/model_closures.py	`100.00% <100.00%> (ø)`
botorch/optim/core.py	`100.00% <100.00%> (ø)`
botorch/optim/fit.py	`100.00% <100.00%> (ø)`
botorch/optim/numpy_converter.py	`100.00% <100.00%> (ø)`
botorch/optim/stopping.py	`100.00% <100.00%> (ø)`
... and 7 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

facebook-github-bot · 2022-10-20T15:39:58Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-10-28T15:47:44Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-11-04T18:29:01Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-11-04T21:24:26Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-11-07T17:01:13Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-11-07T18:19:41Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-11-07T19:15:48Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-11-07T19:51:56Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-11-11T00:12:48Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-11-11T01:38:52Z

This pull request was exported from Phabricator. Differential Revision: D39101211

facebook-github-bot · 2022-11-11T01:52:55Z

This pull request was exported from Phabricator. Differential Revision: D39101211

Summary: X-link: facebook/Ax#1191 Pull Request resolved: #1439 This diff acts as follow-up to the recent model fitting refactor. The previous update focused on the high-level logic used to determine which fitting routines to use for which MLLs. This diff refactors the internal machinery used to evaluate forward-backward passes (producing losses and gradients, respectively) during optimization. The solution we have opted for is to abstract away the evaluation process by relying on closures. In most cases, these closures are automatically constructed by composing simpler, multiply-dispatched base functions. Reviewed By: Balandat Differential Revision: D39101211 fbshipit-source-id: f4ec341228f9f16a327307ff398c3fb8839a3de2

facebook-github-bot · 2022-11-11T17:06:30Z

This pull request was exported from Phabricator. Differential Revision: D39101211

Summary: Pull Request resolved: #1191 X-link: pytorch/botorch#1439 This diff acts as follow-up to the recent model fitting refactor. The previous update focused on the high-level logic used to determine which fitting routines to use for which MLLs. This diff refactors the internal machinery used to evaluate forward-backward passes (producing losses and gradients, respectively) during optimization. The solution we have opted for is to abstract away the evaluation process by relying on closures. In most cases, these closures are automatically constructed by composing simpler, multiply-dispatched base functions. Reviewed By: Balandat Differential Revision: D39101211 fbshipit-source-id: c2058a387fd74058073cfe73c9404d2df2f9b55a

Summary: ## Motivation Removes everything deprecated in pytorch#1439, in version 0.8.0, except for `_get_extra_mll_args`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: pytorch#1995 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs pytorch#1439 Reviewed By: Balandat Differential Revision: D48738275 Pulled By: esantorella fbshipit-source-id: 38f39d185c0cc843f4be7ccd420c0430d3fa1fcd

Summary: ## Motivation Removes everything deprecated in pytorch#1439, in version 0.8.0, except for `_get_extra_mll_args`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: pytorch#1995 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs pytorch#1439 Differential Revision: https://internalfb.com/D48738275 fbshipit-source-id: 4cb19467d42d782c4abe95810e48428c193bef99

Summary: ## Motivation Removes everything deprecated in pytorch#1439, in version 0.8.0, except for `_get_extra_mll_args`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: pytorch#1995 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs pytorch#1439 Reviewed By: Balandat Differential Revision: D48738275 Pulled By: esantorella fbshipit-source-id: 828db010206db8d83071f53576ffba5b6dd49ea7

…h_torch from BoTorch (pytorch#1995) Summary: ## Motivation Removes most of what was deprecated in pytorch#1439, in version 0.8.0, except for `_get_extra_mll_args` and `fit_gpytorch_model`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: pytorch#1995 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs pytorch#1439 Differential Revision: https://internalfb.com/D48738275 fbshipit-source-id: 699d54a6382bd18996624d45a0ba1a564d2e0390

…h_torch from BoTorch (pytorch#1995) Summary: ## Motivation Removes most of what was deprecated in pytorch#1439, in version 0.8.0, except for `_get_extra_mll_args` and `fit_gpytorch_model`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: pytorch#1995 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs pytorch#1439 Differential Revision: https://internalfb.com/D48738275 fbshipit-source-id: 382aacf301178208f21c18071d104085e0f7f73a

…h_torch from BoTorch (pytorch#1995) Summary: ## Motivation Removes most of what was deprecated in pytorch#1439, in version 0.8.0, except for `_get_extra_mll_args` and `fit_gpytorch_model`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: pytorch#1995 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs pytorch#1439 Differential Revision: https://internalfb.com/D48738275 fbshipit-source-id: bcdf275627537c78243f8a01bd291bed3fc764e8

…h_torch from BoTorch (pytorch#1995) Summary: ## Motivation Removes most of what was deprecated in pytorch#1439, in version 0.8.0, except for `_get_extra_mll_args` and `fit_gpytorch_model`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: pytorch#1995 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs pytorch#1439 Reviewed By: Balandat Differential Revision: D48738275 Pulled By: esantorella fbshipit-source-id: bd9874f91cb90a8745943d2ae64facacc52d7a0e

…h_torch from BoTorch (pytorch#1995) Summary: ## Motivation Removes most of what was deprecated in pytorch#1439, in version 0.8.0, except for `_get_extra_mll_args` and `fit_gpytorch_model`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: pytorch#1995 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs pytorch#1439 Reviewed By: Balandat Differential Revision: D48738275 Pulled By: esantorella fbshipit-source-id: dae2be3547dec7406ad342198521897cf0afdca8

…h_torch from BoTorch (pytorch#1995) Summary: ## Motivation Removes most of what was deprecated in pytorch#1439, in version 0.8.0, except for `_get_extra_mll_args` and `fit_gpytorch_model`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: pytorch#1995 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs pytorch#1439 Reviewed By: Balandat Differential Revision: D48738275 Pulled By: esantorella fbshipit-source-id: cc34165b35429fe3967f635ede73c40c32ff2730

…h_torch from BoTorch (pytorch#1995) Summary: ## Motivation Removes most of what was deprecated in pytorch#1439, in version 0.8.0, except for `_get_extra_mll_args` and `fit_gpytorch_model`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: pytorch#1995 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs pytorch#1439 Differential Revision: https://internalfb.com/D48738275 fbshipit-source-id: ca0b2f52fbbcff8c88a624c7f89c63cb06928f53

…rch (pytorch#1995) Summary: ## Motivation Removes most of what was deprecated in pytorch#1439, in version 0.8.0, except for `_get_extra_mll_args` and `fit_gpytorch_model`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: pytorch#1995 Pull Request resolved: pytorch#2041 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs pytorch#1439 Differential Revision: https://internalfb.com/D50276821 fbshipit-source-id: c11ff61ad694e3e4db04ebf338c0745f22a61f79

…rch (pytorch#1995) Summary: ## Motivation Removes most of what was deprecated in pytorch#1439, in version 0.8.0, except for `_get_extra_mll_args` and `fit_gpytorch_model`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: pytorch#1995 Pull Request resolved: pytorch#2041 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs pytorch#1439 Differential Revision: https://internalfb.com/D50276821 fbshipit-source-id: 2c90b1b254a5246160b9848812155de68b2a00ce

…rch (#1995) Summary: ## Motivation Removes most of what was deprecated in #1439, in version 0.8.0, except for `_get_extra_mll_args` and `fit_gpytorch_model`. Removal was straightforward since the functionality was not used and relevant unit tests were clearly labeled and self-contained. ### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)? Yes Pull Request resolved: #1995 Pull Request resolved: #2041 Test Plan: Existing units; make sure codecov has not regressed from deleting tests. ## Related PRs #1439 Reviewed By: saitcakmak Differential Revision: D50276821 Pulled By: esantorella fbshipit-source-id: 45c8f082cdd00cb3bb78a342e38e3f0e751cf56f

facebook-github-bot added CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported labels Oct 3, 2022

j-wilson mentioned this pull request Oct 3, 2022

[Feature Request] Minibatch training when mll is an _ApproximateMarginalLogLikelihood #1438

Open

3 tasks

Balandat reviewed Oct 4, 2022

View reviewed changes

jacobrgardner reviewed Oct 4, 2022

View reviewed changes

botorch/optim/fit.py Outdated Show resolved Hide resolved

j-wilson mentioned this pull request Oct 9, 2022

Botorch closures (#1439) facebook/Ax#1191

Closed

j-wilson changed the title ~~Loss closures, fit_gpytorch_torch, fit_gyptorch_mll dispatch for ApproximateGPs~~ Botorch closures Oct 20, 2022

facebook-github-bot closed this in 6239291 Nov 11, 2022

esantorella mentioned this pull request Aug 28, 2023

Remove everything deprecated in closures refactor #1995

Closed

esantorella mentioned this pull request Oct 13, 2023

Remove numpy_converter functions, fit_gpytorch_scipy, fit_gpytorch_torch #2041

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Botorch closures #1439

Botorch closures #1439

j-wilson commented Oct 3, 2022

facebook-github-bot commented Oct 3, 2022

facebook-github-bot commented Oct 3, 2022

Balandat Oct 4, 2022

Balandat Oct 4, 2022

j-wilson Oct 4, 2022

Balandat Oct 4, 2022

j-wilson Oct 4, 2022

jacobrgardner Oct 4, 2022

j-wilson Oct 4, 2022

Balandat Oct 4, 2022

j-wilson Oct 4, 2022

Balandat Oct 4, 2022

jacobrgardner commented Oct 4, 2022

facebook-github-bot commented Oct 4, 2022

facebook-github-bot commented Oct 5, 2022

facebook-github-bot commented Oct 10, 2022

codecov bot commented Oct 10, 2022 •

edited

Loading

facebook-github-bot commented Oct 20, 2022

facebook-github-bot commented Oct 28, 2022

facebook-github-bot commented Nov 4, 2022

facebook-github-bot commented Nov 4, 2022

facebook-github-bot commented Nov 7, 2022

facebook-github-bot commented Nov 7, 2022

facebook-github-bot commented Nov 7, 2022

facebook-github-bot commented Nov 7, 2022

facebook-github-bot commented Nov 11, 2022

facebook-github-bot commented Nov 11, 2022

facebook-github-bot commented Nov 11, 2022

facebook-github-bot commented Nov 11, 2022

		data_loader: Convience keyword for passing in a DataLoader instance or dict of
		keyword arguments passed to `get_data_loader` to obtain one. May only be

		if isinstance(torch_optimizer, (type, partial)):
		torch_optimizer = torch_optimizer(params=list(param_dict.values()))

Botorch closures #1439

Botorch closures #1439

Conversation

j-wilson commented Oct 3, 2022

facebook-github-bot commented Oct 3, 2022

facebook-github-bot commented Oct 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobrgardner commented Oct 4, 2022

facebook-github-bot commented Oct 4, 2022

facebook-github-bot commented Oct 5, 2022

facebook-github-bot commented Oct 10, 2022

codecov bot commented Oct 10, 2022 • edited Loading

Codecov Report

facebook-github-bot commented Oct 20, 2022

facebook-github-bot commented Oct 28, 2022

facebook-github-bot commented Nov 4, 2022

facebook-github-bot commented Nov 4, 2022

facebook-github-bot commented Nov 7, 2022

facebook-github-bot commented Nov 7, 2022

facebook-github-bot commented Nov 7, 2022

facebook-github-bot commented Nov 7, 2022

facebook-github-bot commented Nov 11, 2022

facebook-github-bot commented Nov 11, 2022

facebook-github-bot commented Nov 11, 2022

facebook-github-bot commented Nov 11, 2022

codecov bot commented Oct 10, 2022 •

edited

Loading