-
Notifications
You must be signed in to change notification settings - Fork 444
sampling.pathwise #1463
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
sampling.pathwise #1463
Conversation
This pull request was exported from Phabricator. Differential Revision: D40662482 |
Codecov Report
@@ Coverage Diff @@
## main #1463 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 155 165 +10
Lines 13796 14277 +481
==========================================
+ Hits 13796 14277 +481
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Thanks for putting this up - it seems to work like a charm. One question regarding the sampling: It raises a It seems to work fine (as in that it runs if I remove the |
@hvarfner Good catch. It should work for FixedNoiseGaussianLikelihoods/FixedNoiseGPs. I simply forgot that Other than that, there's a trivial dispatch in |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
@j-wilson Awesome, thanks! After some more usage, it seems like the samplers consistently expect |
@hvarfner Should be fixed; just needed to specify
|
This pull request was exported from Phabricator. Differential Revision: D40662482 |
sampling.pathwise (meta-pytorch#1463)
This pull request was exported from Phabricator. Differential Revision: D40662482 |
1 similar comment
This pull request was exported from Phabricator. Differential Revision: D40662482 |
self.initialize(input_shape=x.shape[-1:]) | ||
|
||
x = x if self.input_transform is None else self.input_transform(x) | ||
z = x @ self.weight.transpose(-2, -1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% certain, but it seems like there's a dimension too many in the weights.In the MultiOutput / FullyBayesian case, we get:
self.weight: num_outputs * 1 * num_dim * num_features
x: num_outputs * num_queries * num_features`
'z: x @ self.weight.transpose(-2, -1) -->
num_outputs * num_outputs * num_queries * num_features
Not sure how to actually fix this, but the result does not seem to be the desired one.
Finding: self.weight seems to have one dimension too many in these cases, both in AffinePath and GeneralizedLinearBasis. The patchwork fix:
self.weight = self.weight.squeeze(1)
here
self.weight = self.weight.squeeze(2)
in paths.py#34
This does not affect the single output case as far as I can tell.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! Thanks for pointing this out. I think there's a good chance you're right that there's something funny going here. Are you using the latest version of this PR (there was a bug in a previous version where num_outputs
was not being passed)? If so I would sooner expect
self.weight: batch_shape x num_outputs x num_dim x num_features
,
which in many cases would resolve to
self.weight: num_outputs x num_outputs x num_dim x num_features
.
Basically, my code (erroneously) views model.batch_shape
and multi-output dimensions as separate things whereas BoTorch absorbs the multi-output dimension as part of the batch shape. Time allowing, I'll look into this and hopefully get back to you with a fix. If you happen to have a simple example that reproduces the issue that would be helpful.
Addendum: model.batch_shape
does not include the output dimension, so this part was actually correct. The issue is that, e.g., model.covar_module.batch_shape
does include the output dimension. For now, I've addressed this by always relying on model.covar_module.batch_shape
. @Balandat If possible, it would be nice to unify this convention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hvarfner This issue should be resolved in the latest version of the PR. Note that model.covar_module.batch_shape
now dictates the batch shape. Among other things, this means that we no longer maintain an explicit output dimension in the non-batch setting.
This pull request was exported from Phabricator. Differential Revision: D40662482 |
Great stuff! This is very useful for applications that my group and I at SLAC National Accelerator Laboratory are working on. Are there plans to release a version with these pathwise sampling methods included anytime soon? Thanks! |
Hi @dylanmkennedy. Sorry for the delay. Yes, I plan on finalizing an initial release of the pathwise sampling code in the near future (hopefully later this week). |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
11 similar comments
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
4 similar comments
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request was exported from Phabricator. Differential Revision: D40662482 |
Summary: Pull Request resolved: #1463 This PR contains code for efficiently sampling functions from (approximate) GP priors and posteriors. The functionality introduced here is largely exposed via three high-level methods: - `draw_kernel_features`: Generates a feature map that represents (or approximates) a kernel. - `draw_kernel_feature_paths`: Draws functions from a Bayesian-linear-model-based approximation to a GP prior. By default, uses random Fourier features (RFF) to represent stationary priors. - `draw_matheron_paths`: Generates draws from an approximate GP posterior using Matheron's rule. By default, this method combines draws from an RFF-based approximate prior with exact Gaussian updates. For details, see [[Wilson et al., 2020]](https://arxiv.org/abs/2002.09309#) and [[Wilson et al., 2021]](https://arxiv.org/pdf/2011.04026.pdf). Please let us know if you run into issues. As always, contributions are welcomed. Reviewed By: saitcakmak Differential Revision: D40662482 fbshipit-source-id: fdbcfc1453b2153816fdf169357ec106b6a4926a
This pull request was exported from Phabricator. Differential Revision: D40662482 |
This pull request has been merged in b3d3074. |
This PR contains code for efficiently sampling functions from (approximate) GP priors and posteriors. The functionality introduced here is largely exposed by three high-level methods:
gen_kernel_features
: Generates a feature map that represents (or approximates) a kernel.draw_kernel_feature_paths
: Draws functions from a Bayesian-linear-model-based approximation to a GP prior. By default, uses random Fourier features (RFF) to represent stationary priors.draw_matheron_paths
: Generates draws from an approximate GP posterior using Matheron's rule. By default, this method combines draws from an RFF-based approximate prior with exact Gaussian updates. For details, see [Wilson et al., 2020] and [Wilson et al., 2021].Please let us know if you run into issues. As always, contributions are welcomed.