MarginalApprox doesn't allow non-constant covariance parameters or inducing point locations in v4 #5922
Comments
|
@bwengals @lucianopaz ths looks like either the refactoring of |
|
I made a copy of the if is_observed:
return pm.Potential(
name,
self._build_marginal_likelihood_logp(
y,
X,
Xu,
self.sigma,
jitter
),
**kwargs,
)With this change I can successfully run an example where both the hyperparameters and inducing points are optimized: x = np.linspace(0., 1., 10)
xu_init = np.linspace(0., 1., 5)
y = np.sin(x)
with pm.Model():
sigma_gp = pm.HalfNormal('sigma_gp', sigma=1.)
l = pm.HalfNormal('l', sigma=0.1)
cov = sigma_gp**2 * pm.gp.cov.Matern32(1, ls=[l])
gp = MarginalApprox(cov_func=cov, approx='VFE')
sigma_noise = pm.HalfNormal('sigma_noise', sigma=1.)
xu = pm.Flat('xu', shape=(5, 1), initval=xu_init[:,None])
gp.marginal_likelihood('like', X=x[:,None], y=y,
noise=sigma_noise, Xu=xu)
maxpost = pm.find_MAP()
print(maxpost)which outputs {'sigma_gp_log__': array(0.001806), 'l_log__': array(-2.30458196), 'sigma_noise_log__': array(0.00020157), 'xu': array([[0.00267675],
[0.24694185],
[0.49314526],
[0.75139309],
[0.98770933]]), 'sigma_gp': array(1.00180763), 'l': array(0.09980051), 'sigma_noise': array(1.00020159)}Now, I find this to be an implausible result even for this entirely artifical problem. (Somehow the arbitrary initial values for all these variables just happen to be very close to the MAP values?) However, find_MAP has often been producing bad results in v4 (see #5923), so I think that is a separate issue. |
|
The error means the DensityDist Dist logp function is likely referencing other model variables that are not passed as explicit arguments to the logp function and to the DensityDist class. They seem to come from Whether the DensityDist is the right object for this or not, depends on whether one needs to to sample the variable defined by the DensityDist. Potentials do not have variables being sampled (although one can achieve the same by combining it with Lines 685 to 787 in f5d3431 |
|
Apologies for missing this!
By that measure, I think the answer is yes. This is really just a sort of weird approximated logp of a multivariate normal. Should have provided a Is the fix just needing |
I don't have a good mental model of what this does to answer, but it seems like... no? Aeppl is about converting graphs of RandomVariables to respective closed form logp graphs.
Yes. It just needs all non local (non constant) TensorVariables that are used in the logp function to be passed as explicit inputs. You just need to be careful because you are working with a Multivariate DensityDist to make sure you pass the right This test shows how the DensityDist is used to reimplement a mvnormal pymc/pymc/tests/test_distributions.py Line 3148 in c8db06b |
|
Thanks, RE aeppl I see, then no. That should be a pretty easy fix then. Thanks for the test, that'll help too |
|
Actually, this might be a bit tricker, and maybe is more aeppl-ish. Is there a way around this? It probably wouldn't be a good idea to try and unpack the RVs that might be inside
Since aeppl gives logp implementations for different RVs, I think it might make sense here? |
Why not? |
|
A Just to be sure, I should have been more specific when I said "the sort of thing that's better in aeppl". What I'm picturing is instead of I'm not super familiar with aeppl, so apologies if I'm not quite getting it. |
|
The only issue I see here is that any RV used in the logp needs to be an input to the DensityDist or whatever Distribution class is used instead. There is no way around that, because Aeppl needs to know all the RV dependencies in advance before the logp methods are called. DensityDist makes sense if the logp function itself changes across different GPs, otherwise you could create a single GPRV (or GPMvNormalRV or whatever you want to call it) which has a single logp function. Regardless, the constraint that any RV used in the logp must be an explicit input to the Densitydist/new RV remains the same. Is the set of RVs used in the logp impossible to know in advance / when you are calling the DensityDist? |
|
Sounds like the problems arise from the GP module creating lazy Cov/ Mean Python objects instead of Aesara graphs which would demand explicit inputs from the get-go. |
|
I am getting the same error with the MarginalSparse Process. Am I correct in concluding that this makes it impossible to do multivariate SGPR with PyMC4? |
Description of your problem
The following code that worked in PyMC3 (and works in v4 if
Marginalis used instead ofMarginalApprox) is no longer functional:Complete error traceback
This is probably related to some degree to the
DensityDistchanges and to #5024. Note that the following case wheresigma_noiseis the only random variable works fine:yielding
{'sigma_noise_log__': array(-2.8405184), 'sigma_noise': array(0.05839539)}I'm guessing that this is why this error was not caught earlier.
MarginalApproxneeds a test to be added with non-constant length scales.There's a completely different error if the inducing point locations are non-constant. Example code:
Complete error traceback
I don't really understand this error, since it's not really clear to me when or how the shape information is propagated through either the Aesara variables or PyMC wrappers. So I'm not sure if this error has the same underlying cause as the other or not.
I will say that this regression is rather disappointing, since being unable to tune either the hyperparameters or the inducing points makes
MarginalApproxmuch less useful.Versions and main components
The text was updated successfully, but these errors were encountered: