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

[Bug] AffineInputTransform does not check dimensions after training #2509

Closed
slishak-PX opened this issue Sep 5, 2024 · 2 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@slishak-PX
Copy link
Contributor

🐛 Bug

AffineInputTransform does not always check the shape of tensors when transforming or untransforming, which can result in confusing behaviour: when a tensor of shape [..., 1] is passed and it expects a tensor of shape [..., d], it will broadcast it up to the expected dimension when (un)transforming.

To reproduce

** Code snippet to reproduce **

import torch
from botorch.models import SingleTaskGP
from botorch.models.transforms import Normalize

n_inputs = 4
n_outputs = 1
n_train = 256
n_test = 16
device = torch.device("cpu")

train_x = torch.rand(n_train, n_inputs, dtype=torch.float64, device=device)
train_y = torch.randn(n_train, n_outputs, dtype=torch.float64, device=device)

# Note d=1 instead of n_inputs
test_x_incorrect_dim = torch.randn(n_test, 1, dtype=torch.float64, device=device)

gp_norm = SingleTaskGP(train_x, train_y, input_transform=Normalize(n_inputs))

# This doesn't raise an exception because the input_transform doesn't check the input dimensionality
# and ends up broadcasting up to n_inputs
posterior_incorrect = gp_norm.posterior(test_x_incorrect_dim)

# This shows the issue
print(gp_norm.input_transform.transform(test_x_incorrect_dim).shape)
# torch.Size([16, 4])

# This fails correctly with a sensible error message from GPyTorch
gp = SingleTaskGP(train_x, train_y)
posterior_incorrect_fails = gp.posterior(test_x_incorrect_dim)

** Stack trace/error message **
No error, which is the problem!

Expected Behavior

self._check_shape(X) should always be called during _transform and _untransform.

@slishak-PX slishak-PX added the bug Something isn't working label Sep 5, 2024
@esantorella esantorella self-assigned this Sep 5, 2024
@esantorella
Copy link
Member

Good catch, thanks for reporting! We'll put in a fix.

esantorella added a commit to esantorella/botorch that referenced this issue Sep 6, 2024
…, even in eval mode

Summary:
Context: pytorch#2509 gives a clear overview

This PR:
* Checks the shape of the `X` provided to an `AffineInputTransform` when it transforms the data, regardless of whether it is updating the coefficients.

Makes some unrelated changes:
* Fixes the example in the docstring for `batched_multi_output_to_single_output`
* fixes an incorrect shape in `test_approximate_gp`
* Makes data and transform batch shapes match in `TestConverters`, since those usages will now (appropriately) error

Differential Revision: D62318530
esantorella added a commit to esantorella/botorch that referenced this issue Sep 6, 2024
…, even in eval mode (pytorch#2510)

Summary:
Pull Request resolved: pytorch#2510

Context: pytorch#2509 gives a clear overview

This PR:
* Checks the shape of the `X` provided to an `AffineInputTransform` when it transforms the data, regardless of whether it is updating the coefficients.

Makes some unrelated changes:
* Fixes the example in the docstring for `batched_multi_output_to_single_output`
* fixes an incorrect shape in `test_approximate_gp`
* Makes data and transform batch shapes match in `TestConverters`, since those usages will now (appropriately) error

Differential Revision: D62318530
esantorella added a commit to esantorella/botorch that referenced this issue Sep 6, 2024
…, even in eval mode (pytorch#2510)

Summary:
Pull Request resolved: pytorch#2510

Context: pytorch#2509 gives a clear overview

This PR:
* Checks the shape of the `X` provided to an `AffineInputTransform` when it transforms the data, regardless of whether it is updating the coefficients.

Makes some unrelated changes:
* Fixes the example in the docstring for `batched_multi_output_to_single_output`
* fixes an incorrect shape in `test_approximate_gp`
* Makes data and transform batch shapes match in `TestConverters`, since those usages will now (appropriately) error

Reviewed By: saitcakmak

Differential Revision: D62318530
esantorella added a commit to esantorella/botorch that referenced this issue Sep 6, 2024
…, even in eval mode (pytorch#2510)

Summary:
Pull Request resolved: pytorch#2510

Context: pytorch#2509 gives a clear overview

This PR:
* Checks the shape of the `X` provided to an `AffineInputTransform` when it transforms the data, regardless of whether it is updating the coefficients.

Makes some unrelated changes:
* Fixes the example in the docstring for `batched_multi_output_to_single_output`
* fixes an incorrect shape in `test_approximate_gp`
* Makes data and transform batch shapes match in `TestConverters`, since those usages will now (appropriately) error

Reviewed By: saitcakmak

Differential Revision: D62318530
esantorella added a commit to esantorella/botorch that referenced this issue Sep 6, 2024
…, even in eval mode (pytorch#2510)

Summary:
Pull Request resolved: pytorch#2510

Context: pytorch#2509 gives a clear overview

This PR:
* Checks the shape of the `X` provided to an `AffineInputTransform` when it transforms the data, regardless of whether it is updating the coefficients.

Makes some unrelated changes:
* Fixes the example in the docstring for `batched_multi_output_to_single_output`
* fixes an incorrect shape in `test_approximate_gp`
* Makes data and transform batch shapes match in `TestConverters`, since those usages will now (appropriately) error

Reviewed By: saitcakmak

Differential Revision: D62318530
facebook-github-bot pushed a commit that referenced this issue Sep 9, 2024
…, even in eval mode (#2510)

Summary:
Context: #2509 gives a clear overview

This PR:
* Checks the shape of the `X` provided to an `AffineInputTransform` when it transforms the data, regardless of whether it is updating the coefficients.

Makes some unrelated changes:
* Fixes the example in the docstring for `batched_multi_output_to_single_output`
* fixes an incorrect shape in `test_approximate_gp`
* Makes data and transform batch shapes match in `TestConverters`, since those usages will now (appropriately) error

Pull Request resolved: #2510

Reviewed By: saitcakmak

Differential Revision: D62318530

Pulled By: esantorella

fbshipit-source-id: eaa8b0410c49b17d6abbe1391bbb0750313aea23
@esantorella
Copy link
Member

Closed by #2510

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants