Skip to content

RFF/linear prediction strategy fix (take 2)#2681

Merged
gpleiss merged 4 commits into
mainfrom
linear_pred_strategy
Nov 19, 2025
Merged

RFF/linear prediction strategy fix (take 2)#2681
gpleiss merged 4 commits into
mainfrom
linear_pred_strategy

Conversation

@gpleiss

@gpleiss gpleiss commented Nov 17, 2025

Copy link
Copy Markdown
Member

Attempt 2 at #2671 , but this time adding a naive fantasy strategy for BoTorch models.

The fantasy strategy is O(d^2 (n+m)), so it is no more efficient than re-conditioning the model. Without writing custom cuda kernels, it is unlikely to be more efficient.

@Balandat Balandat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Test failure due to flakiness maybe?

cc @sdaulton as this affects BoTorch's FullyBayesianLinearSingleTaskGP (will test this separately)

Comment thread gpytorch/models/exact_prediction_strategies.py Outdated
Comment thread gpytorch/models/exact_prediction_strategies.py Outdated
Comment thread gpytorch/models/exact_prediction_strategies.py Outdated
Comment thread gpytorch/models/exact_prediction_strategies.py
train_factor = lt.root.to_dense()
train_train_covar = self.lik_train_train_covar
inner_term = (
train_train_covar = self.lik_train_train_covar.evaluate_kernel() # HOTFIX here: need to resolve lazy eval!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@gpleiss what's the deal with this? Why exactly is this necessary? This seems to be causing some other test failure here when running on a GPU, presumably due to changed numerical behavior: https://github.com/meta-pytorch/botorch/blob/main/test/models/test_fully_bayesian.py#L834

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We need to resolve the LazyEvaluatedKernelTensor, because otherwise the $\boldsymbol K + \sigma^2 I$ matrix will be an AddedDiagLinearOperator(LazyEvaluatedKernelTensor, DiagLinearOperator) rather than a LowRankRootAddedDiagLinearOperator(LowRankRootLinearOperator, DiagLinearOperator). The latter is necessary to get the structure we want.

I can remove this evaluate_kernel if I add the following line before calling the super constructor:

train_prior_dist = train_prior_dist.__class__(
    train_prior_dist.mean, train_prior_dist.lazy_covariance_matrix.evaluate_kernel()
)

(which I originally added after calling the super constructor; moving it before is probably a better idea). I'm not sure if that will fix the numerical issues on the GPU test though?

@gpleiss
gpleiss force-pushed the linear_pred_strategy branch from 12076a7 to fadf30c Compare November 18, 2025 19:19

@Balandat Balandat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@gpleiss
gpleiss merged commit d6f02cc into main Nov 19, 2025
7 checks passed
@gpleiss
gpleiss deleted the linear_pred_strategy branch November 19, 2025 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants