search

statrs-dev / statrs

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

feature: Implement Continuous<Vec<f64>> for MultivariateNormal #199

Closed saona-raimundo closed 2 months ago

saona-raimundo commented 5 months ago

Solves #194

The problem was the public dependency on nalgebra::Dvector: Although statrs does not update its dependencies, users should easily sample from MultivariateNormal.

The users were using an updated version of nalgebra and the types had clashes. Therefore, this implements the same trait for a type with guaranteed backward compatibility: Vec.

Maintaining or not the nalgebra::DVector in the public API is an open question that we did not touch.

YeungOnion commented 2 months ago

Thanks for identifying a good solution here. I'm good to merge this, @henryjac thoughts?

YeungOnion commented 2 months ago

Hmm, I believe I merged this in a rush. I'd yet to get input here on #209 which reintroduces the coupling to internal and at API usage of nalgebra and I was considering the option for an multidimensional_nalgebra feature to allow using this without using an internal nalgebra which would be a taller order.

saona-raimundo commented 2 months ago

As far as I could see, the problem of #194 (requiring the user to use possibly more than one version of nalgebra) is also present on #209 If this is the case, then I see no problem in accepting both interfaces: Vec can always be used, while nalgebra can be used easily if there is no dependency clashes. Before we make it efficient or convenient, it should "just work", and I think Vec is needed in this case.

YeungOnion commented 2 months ago

I think you're right that both will work for now as it (and similar for later ContinuousCDF) solves the usage of trait Continuous.

However, I think the traits that return OVector (e.g. MeanN, VarianceN, rand::distributions::Distribution) would return library constrained version of OVector. That sound right? Maybe that'd be another issue?

saona-raimundo commented 2 months ago

Ah! You are right. I would say they are different kind of problems. One thing is the user needing to input an nalgebra struct of a specific version, while another is statrs returning an nalgebra struct of a specific version. They both introduce a possible dependency problem, but they have different solutions.

I have not tested it but the second might not be much of a problem. If the struct that statrs returns has a method to construct a Vec with the underlying data, and the user does not need to add a dependency for that, then this is not much of a problem. We can mention it in the documentation.

YeungOnion commented 2 months ago

Ah yes, it's on the other side. I'll write it as a separate issue, thanks for your input!