pymc icon indicating copy to clipboard operation
pymc copied to clipboard

MarginalSparse shouldn't use DensityDist internally

Open lucianopaz opened this issue 2 years ago • 2 comments

Description of your problem

The MarginalSparse Gaussian Process uses DensityDist internally to compute the marginal_likelihood (here). We should refactor that code to use a proper RandomVariable instead

Versions and main components

  • PyMC3 Version: 4.0
  • Aesara/Theano Version: 2.2.2
  • Python Version: 3.8.5
  • Operating system: Ubuntu 18.04
  • How did you install PyMC3: (conda/pip) pip

lucianopaz avatar Sep 24 '21 16:09 lucianopaz

This would be nice to do. It would allow pm.sample_posterior_predictive to work correctly over the original input domain X (so, to see the fit) without the need to append an additional gp.conditional rv. Is there another benefit?

In the interest of keeping #5055 smaller, I think it should be done after that's in, or branched off of it in a separate PR. I might be mistaken, but I think it's more of an enhancement than something broken that must be fixed before v4.

It also makes me wonder if other GP implementations would be better refactored into RandomVariables as well.

bwengals avatar Oct 28 '21 20:10 bwengals

@bwengals Are you interested in pursuing this one? Or should we add a "help wanted" label?

ricardoV94 avatar Feb 03 '22 14:02 ricardoV94

Closed by #6076

michaelosthege avatar Oct 09 '22 23:10 michaelosthege