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

Cov arguments in TP and MvStudentT #2865

Closed
jordan-melendez opened this issue Feb 21, 2018 · 5 comments · Fixed by #6068
Closed

Cov arguments in TP and MvStudentT #2865

jordan-melendez opened this issue Feb 21, 2018 · 5 comments · Fixed by #6068

Comments

@jordan-melendez
Copy link
Contributor

I am migrating this discussion from Discourse:

... It seems like cov_func and cov are misleading parameter names, since they aren’t actually the covariances of the distributions. Rather, they refer to the Sigma parameter of MvStudentT, which is related to the covariance by nu * Sigma / (nu-2). Thus, here, for example, it looks like apples and oranges are being compared when using the same cov_func in a GP and TP. Can these arguments be changed so that they really are covariances, or at least be better documented so that people know that they aren’t actually what they seem to be?

What is the correct course of action here? Better documentation or changing the functionality to align with the argument names?

@fonnesbeck
Copy link
Member

Ideally, it would be nice to have the option of parameterizing via Sigma or a true covariance (just as we can parameterize a beta with scale and shape parameters or mean and variance).

@bwengals
Copy link
Contributor

Would a good alternative name maybe be scale_func?

@jordan-melendez
Copy link
Contributor Author

I agree on allowing options for parameterizing with a cov_func or something like a scale_func. Does the current MvStudentT actually treat Sigma differently than cov? And if so, how does it handle the cov if nu is less than 2, where it should be undefined? This seems tricky if nu is unobserved, unless cov is always converted back to Sigma behind the scenes.

@michaelosthege
Copy link
Member

This is the oldest open issue right now.
@bwengals is it still relevant?
It would be a good time to rename kwargs now if needed.

@bwengals
Copy link
Contributor

Yes this should still be fixed

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

Successfully merging a pull request may close this issue.

5 participants