Skip to content
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

MarginalSparse shouldn't use DensityDist internally #5024

Closed
lucianopaz opened this issue Sep 24, 2021 · 3 comments
Closed

MarginalSparse shouldn't use DensityDist internally #5024

lucianopaz opened this issue Sep 24, 2021 · 3 comments

Comments

@lucianopaz
Copy link
Contributor

lucianopaz commented Sep 24, 2021

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
@bwengals
Copy link
Contributor

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.

@ricardoV94 ricardoV94 modified the milestones: v4.0.0b2, v4.0.0b3 Jan 7, 2022
@ricardoV94 ricardoV94 modified the milestones: v4.0.0b3, v4.0.0 Feb 3, 2022
@ricardoV94
Copy link
Member

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

@michaelosthege
Copy link
Member

Closed by #6076

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants