statrs-dev / statrs

Statistical computation library for Rust
https://docs.rs/statrs/latest/statrs/
MIT License
582 stars 83 forks source link

Multivariate students t distribution #176

Closed henryjac closed 4 weeks ago

henryjac commented 1 year ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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.

YeungOnion commented 7 months ago

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 commented 5 months ago

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

FreezyLemon commented 4 weeks ago

Can't this be closed now that #266 is merged?