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

Mutable/Movable parameters for multivariate normal #180

Open
limads opened this issue Jan 5, 2023 · 1 comment
Open

Mutable/Movable parameters for multivariate normal #180

limads opened this issue Jan 5, 2023 · 1 comment

Comments

@limads
Copy link

limads commented Jan 5, 2023

Hello, I'm studying the possibility of using this crate for Markov Chain Monte Carlo (MCMC) based inference. In this use case, the log-density of a distribution is evaluated repeatedly at different parameter values. To do that currently, the crate requires re-creating the distributions at each iteration. This isn't much of a problem for scalar distributions, but for the multivariate normal, I have to re-allocate the mean vector and covariance matrix at each iteration (since distributions are immutable), which impacts performance.

Allowing the user to re-set the parameters separately would work:

pub fn set_mean(&mut self, mu : &[f64]);
pub fn set_variance(&mut self, var : &[f64]);

But a solution that moves the parameters out of the struct would also work (therefore preserving the intended immutable API):

pub fn take_parameters(self) -> (DVector<f64>, DMatrix<f64>);

Are there any plans to offer something like that?

@limads limads changed the title Mutate/Movable parameters of multivariate normal Mutate/Movable parameters for multivariate normal Jan 5, 2023
@limads limads changed the title Mutate/Movable parameters for multivariate normal Mutable/Movable parameters for multivariate normal Jan 5, 2023
@YeungOnion
Copy link
Contributor

Hi, are you still considering this for MCMC? If so, I think your solution that preserves immutability would be good. I'd be open to a PR for this, please consider making this atop #209 as we plan to merge those changes and those change the multivariate API

Aside as a possibility for an optimization if the math is nice:
If the transformations you make on the covariance can be expressed as operations on the decomposition and its inverse, then consider a take_parameters_and_precomputed as well, which would motivate a new from new_with_precomputed_unchecked (does not verify that LL* = Σ or LP = 1 for provided (μ, Σ), (L, P, |Σ|)) and new_with_precomputed

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

No branches or pull requests

2 participants