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

Use Cholesky decomposition for correlated_values #99

Open
ces42 opened this issue Jun 23, 2019 · 1 comment
Open

Use Cholesky decomposition for correlated_values #99

ces42 opened this issue Jun 23, 2019 · 1 comment

Comments

@ces42
Copy link

ces42 commented Jun 23, 2019

Trying to do by hand exactly what correlated_values(nom_values, covariance_mat) does I thought of a different approach than the one used in the current implementation:

n = len(nom_values)
L = cholesky(covariance_mat)
return nom_values + dot(L, uarray(zeros(n), ones(n))

This essentially does the same as correlated_values (except for the tags and it returns a array instead of a tuple). It should be faster than diagonalizing the covariance matrix and some quick tests suggest that it might be numerically more precise:
For nom_values = ones(10) and covariance_mat = scipy.linalg.hilbert(10) the reconstructed covariance matrix using covariance_matrix only differs in 15 using cholesky while the current implementation differs in 78 entries.

Is there a reason why the current implementation uses diagonalization instead of cholesky? (If there is no such reason I would be happy to write a pull request)

P.S. I really like this package and it's saving me a ton of time in my lab course :)

@lebigot
Copy link
Collaborator

lebigot commented Jun 27, 2019

That's an interesting point: thanks for sharing!

I feel that what you propose is the better way, indeed. A pull request would be much appreciated.

I am glad that you find this library useful!

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