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

Multivariate students t distribution #176

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

henryjac
Copy link
Contributor

Implementation of Multivariate students t distribution in a very similar way to Multivariate normal.
Includes sampling, mean, covariance, mode, pdf and log pdf functions.

Testing with exact values from python scipy.stats.multivariate_t functions. Large degrees of freedoms does not work yet

@henryjac henryjac closed this Feb 27, 2024
@henryjac henryjac deleted the multivariate_t branch February 27, 2024 21:08
@henryjac
Copy link
Contributor Author

henryjac commented Mar 2, 2024

I put you as a reviewer @YeungOnion, but you don't have to review this PR if you don't want to, but I'm just hesitant of merging my own PR now.

@YeungOnion
Copy link
Contributor

Thanks! I took a first glance.

I see the tests for large DOF issues, did you fix that or is the failure mode visible to a user for cases of "should be close enough, but not guaranteed"

Additionally, you have TODO suggesting more matrices to tests, but what else might you add? If I recall correctly, real positive semi-definite is a bijection to real symmetric which means any other tests would really be testing the precision of nalgebra, no?

@henryjac
Copy link
Contributor Author

henryjac commented Mar 3, 2024

About the large DOF issue, I don't remember exactly what I thought about back then, but as I see now the correspondence between mutlivariate normal and mutlivariate t for DOF quite high should go to zero (as it does for the f64::INFINITY test), but the exact difference in value for finite DOF isn't really known to me.

For the other point, more tests should increase the confidence to the user. And any real symmetric matrix is not necessarily positive semi-definite (we are only interested in positive definite though), take
$A = \begin{bmatrix}0&1\\1&0\end{bmatrix}$
which has eigenvalues $\lambda = \pm 1$, so it isn't definite.

I am also doing some more minor changes, but should be good soon.

Also, I don't know the best place to discuss it openly, but we should define what we wish to do with existing PR and issues and in what direction to take the crate.

Also improves documentation for multivariate t minorly.
@YeungOnion
Copy link
Contributor

Well it's certainly not necessary to aim for better than scipy's implementation, and you've tested the asymptotic behavior.

Regarding the theorem on symmetric matrices, I certainly recalled that wrong.

Oh yeah, adding tests definitely adds confidence, I think that's great. The test you have, including the one you most recently added seems sufficient for implementing a new future. Perhaps opening an issue that's good for first-time contributors to add tests might be valuable.

I think you're right about an open place to discuss. Perhaps this is a little meta but I think we can open an issue to discuss direction and we can update the readme with any goals we (the community) establish.

@YeungOnion
Copy link
Contributor

@henryjac do you want to update this to use the static allocator syntax with nalgebra before merging #209?

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

Successfully merging this pull request may close these issues.

None yet

2 participants