-
Notifications
You must be signed in to change notification settings - Fork 24
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 when computing the mean of marginal variational q(f) #55
Comments
I should have pointed that what I say is only needed when we don't use the whitened representation of inducing points. However, for this general case I still don't see where Gpflow's.conditional considers |
We discussed this question on slack, copied here for posterity: Is that not just a reparametrization? we learn the mean of q(u), so it shouldn't make a difference [except perhaps for initialization] whether we split it up into "K_xz Kzz_inv (m - mean_function(Z))" or "K_xz Kzz_inv m'" (if m['] is just a learned parameter)? so in the DGP example with an identity mean function, the way it's coded at the moment should mean that if you initialize q_mu=zeros then the approximate posterior mean should be exactly the identity function (i.e., q_mu encodes "change away from the mean function", not "function value itself") |
@jmaronas replied:
|
Describe the bug
I think there is a bug when computing the mean of q(f) = \int p(f|u)q(u), because the mean function evaluated at the inducing point locations is not added.
To reproduce
When computing q(f), the code is given by this line here:
GPflux/gpflux/layers/gp_layer.py
Line 257 in e05d7ba
Note that the function conditional computes, for the mean
mean_cond
: K_xz K_zz_inv m . After, \mu(x) is added in the return methodGPflux/gpflux/layers/gp_layer.py
Line 268 in e05d7ba
However, I am quite sure that the variational mean is computed as:
K_xz K_zz_inv m + mu(x) -K_xz K_zz_inv mu(z),
which means that for mean functions different from the zero mean function, a term -K_xz K_zz_inv mu(z) has to be added to the variational mean.
This could be easily solved ( I think ) by calling the conditional as follows:
I have been checking the source code from Gpflow and it does not look like the term K_xz K_zz_inv mu(Z) is being considered.
In case this bug is confirmed, I will open the issue in GPFLOW as well, because the SVGP model, for example, has the same problem.
The text was updated successfully, but these errors were encountered: