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

[Feature Request] Fantasize SingleTaskVariationalGP and/or HeteroskedasticSingleTaskGP #1459

Closed
JordanJar8 opened this issue Oct 20, 2022 · 4 comments
Labels
documentation enhancement New feature or request

Comments

@JordanJar8
Copy link

🚀 Feature Request

An implementation of fantasize method for SingleTaskVariationalGP and/or HeteroskedasticSingleTaskGP

Motivation

Is your feature request related to a problem? Please describe.
I'm trying to do some active learning with a SingleTaskVariationalGP and HeteroskedasticSingleTaskGP on a heteroskedastic regression task (~20 000 samples expected at the end) using qNegIntegratedPosteriorVariance as acquisition function. It requires to fantasize the surrogate model to compute fast posterior covariances but this method isn't implemented yet.

Pitch

Describe the solution you'd like
An implementation of fantasize method that return fantasy model for SingleTaskVariationalGP and HeteroskedasticSingleTaskGP

Describe alternatives you've considered
I've tried to implement myself fantasize method for both models but didn't achieve yet. A little help in how BoTorch fantasize works would be very much appreciated :)

Are you willing to open a pull request? (See CONTRIBUTING)
Yes, why not. I've already tried to modify botorch lib on my PC.

Additional context

None

@JordanJar8 JordanJar8 added the enhancement New feature or request label Oct 20, 2022
@saitcakmak
Copy link
Contributor

Hi @JordanJar8. I am not really familiar with the details, but there's some support in GPyTorch for fantasizing variational GPs. This was implemented in cornellius-gp/gpytorch#1874 and a tutorial is found in https://docs.gpytorch.ai/en/stable/examples/08_Advanced_Usage/SVGP_Model_Updating.html#SVGP-Model-Updating

If you need 20k samples, it is unlikely that HeteroskedasticSingleTaskGP or some other ExactGP would scale up that much (I got SingleTaskGP to work with 10-12k samples for some testing, but it gets too close to OOM), so variational GPs might be a better choice. To fantasize with HeteroskedasticSingleTaskGP, I believe you also need to fantasize on the noise model, which makes it a bit more challenging. @Balandat or @Ryan-Rhys might have more thoughts on this.

I also want to point out is that FixedNoiseGP can also handle heteroscedastic noise observations and supports fantasizing out of the box. It might have a bit more difficulty extrapolating far from where the observations are, but if you have a large observation budget, that shouldn't be too much of a problem, I think.

@JordanJar8
Copy link
Author

Hi @saitcakmak. Thanks for your response.

Yes I did look at GPyTorch variational GPs and fantasizing and make it works, but what I'm trying to do is to wrap it in BoTorch to make it works with all the framework, at least with the qNegIntegratedPosteriorVariance acquisition function and optimization.

I have looked at how the SingleTaskVariationalGP and SingleTaskGP are implemented. I still don't understand why SingleTaskGP inherit from BoTorch's GPyTorchModel class and GPytorch's ExactGP class, and SingleTaskVariationalGP inherit from BoTorch's GPyTorchModel class only but has a model attribute of _SingleTaskVariationalGP class (who is a wrapper of GPyTorch's ApproximateGP). I wonder if it is not this difference of construction that makes the qNegIntegratedPosteriorVariance unable to access the different attributes (model, likelihood, etc.) in the same way as it does with an SingleTaskGP.
This is very confuse for me so that's why I'm seeking your help !

@esantorella
Copy link
Member

Hi @JordanJar8 , thanks for reaching out!

SingleTaskVariationalGP should be used differently from BoTorch models. See the documentation here.

I am not a modeling expert, but I can comment on the structure of the code. BoTorch often uses subclassing to share functionality rather than to define a consistent interface. In other words, if X subclasses Y, that doesn't guarantee that you can use an X wherever you can use a Y; it may just be that X benefitted from sharing a method with Y, and other methods may have come along as for the ride. That's why SingleTaskVariationalGP and HeteroskedasticSingleTaskGP have fantasize methods that raise a NotImplementedError, and why SingleTaskVariationalGP doesn't work the same as other subclasses of GPyTorchModel (such as SingleTaskGP) even though it "is a" GPyTorchModel.

When you're trying to figure out how to use a model or acquisition function, I'd recommend looking at usage examples in the tutorials rather than studying the inheritance structure.

You've identified at least one "documentation bug" here -- the docstrings for SingleTaskVariationalGP.fantasize and HeteroskedasticSingleTaskGP.fantasize should make it clear that these methods are not implemented, since the docstrings are currently misleading.

esantorella added a commit to esantorella/botorch that referenced this issue Oct 31, 2022
…discussion! (pytorch#1462)

Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to make BoTorch better.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to BoTorch here: https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md
-->

## Motivation

The `Model` base class has a `fantasize` method, but it isn't actually possible to `fantasize` from all models. For some models, `fantasize` fails with a `NotImplementedError` even though the docstring implies it should work. This is confusing (e.g. pytorch#1459 ). Another disadvantage is that codecov doesn't require tests for all of the many `fantasize` methods that exist, only for the base one -- which can't be called, since it is abstract.

This PR removes the "fantasize" method from the `Model` base class. Instead, there is a `fantasize` function that is called by the few classes that do actually use and/or test `fantasize`.

Pros:
- decreases the "surface area" of BoTorch and prevents misuse of methods that we didn't really intend to exist
- Allows codecov to surface where we aren't actually testing methods (I expect it to fail)

Cons:
-  `fantasize` still exists but doesn't work in `HeteroskedasticSingleTaskGP` because it inherits from `SingleTaskGP` (a further refactor can fix that)
- Could remove `fantasize` from classes where it _should_ exist (despite not being used or tested within BoTorch)
- Somewhat more verbose

### 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#1462

Test Plan:
[x] Codecov
[x] Unit tests should pass
[x] No new Pyre errors introduced

Differential Revision: D40783106

Pulled By: esantorella

fbshipit-source-id: ecdae44b95b11248170fec7b7a9c9c4f2095c9f8
facebook-github-bot pushed a commit that referenced this issue Oct 31, 2022
…discussion! (#1462)

Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to make BoTorch better.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to BoTorch here: https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md
-->

## Motivation

The `Model` base class has a `fantasize` method, but it isn't actually possible to `fantasize` from all models. For some models, `fantasize` fails with a `NotImplementedError` even though the docstring implies it should work. This is confusing (e.g. #1459 ). Another disadvantage is that codecov doesn't require tests for all of the many `fantasize` methods that exist, only for the base one -- which can't be called, since it is abstract.

This PR removes the "fantasize" method from the `Model` base class. Instead, there is a `fantasize` function that is called by the few classes that do actually use and/or test `fantasize`.

Pros:
- decreases the "surface area" of BoTorch and prevents misuse of methods that we didn't really intend to exist
- Allows codecov to surface where we aren't actually testing methods (I expect it to fail)

Cons:
-  `fantasize` still exists but doesn't work in `HeteroskedasticSingleTaskGP` because it inherits from `SingleTaskGP` (a further refactor can fix that)
- Could remove `fantasize` from classes where it _should_ exist (despite not being used or tested within BoTorch)
- Somewhat more verbose

### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)?

Yes

Pull Request resolved: #1462

Test Plan:
[x] Codecov
[x] Unit tests should pass
[x] No new Pyre errors introduced

Reviewed By: saitcakmak

Differential Revision: D40783106

Pulled By: esantorella

fbshipit-source-id: 91cd7ee47efee1d89ae7f65af1ed94a4d88bdbe6
@esantorella
Copy link
Member

Closing as duplicate of #936 (see also #1393). The TLDR is that fantasizing noise models is tricky and we don't have concrete plans for supporting this (but haven't ruled it out either).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants