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

No pivoting for CHOLMOD factors #120

Closed
jkbest2 opened this issue Oct 16, 2020 · 2 comments · Fixed by #175
Closed

No pivoting for CHOLMOD factors #120

jkbest2 opened this issue Oct 16, 2020 · 2 comments · Fixed by #175

Comments

@jkbest2
Copy link

jkbest2 commented Oct 16, 2020

whiten! and unwhiten use chol_lower to extract the lower triangular component of Cholesky decompositions. For SuiteSparse.CHOLMOD.Factor{T}, this extracts the lower triangular factor, but does not account for pivoting. This results in the same problem seen on this Discourse thread.

Using the sparse precision matrix attached to that thread,

using DataFrames, CSV

q_df = CSV.read("precision_matrix.csv")
point_df = CSV.read("mesh_points.csv")
Q = sparse(q_df.i, q_df.j, q_df.q)

Generate a realization,

using Random, SparseArrays, LinearAlgebra
using Distributions
using PDMats

Random.seed!(2)
z = randn(size(Q, 1))

pdQ = PDSparseMat(cholesky(Q))
xldiv = pdQ.chol.UP \ z
xwh = whiten(pdQ, z)

These two realizations should be the same, but they are not.

julia> maximum(xldiv - xwh)
45.00349329625478

This is also clear when they are plotted.

using Plots
pldiv = surface(point_df.x, point_df.y, xqs, camera=(45, 80), title="ldiv")
pwh = surface(point_df.x, point_df.y, xwh, camera=(45, 80), title="whiten")
plot(pldiv, pwh, layout=(1,2), size=(1200, 400), colorbar=false)

The ldiv version results in a random field with the expected structure, while the whiten version does not.

comparison

Downstream, this means that at least rand(::Distributions.MvNormalCanon) is giving incorrect results. I haven't looked closely at other methods in Distributions.jl yet.

@ElOceanografo
Copy link
Contributor

This issue may be worth fixing in PDMats, but note that the unwhiten_winv! method for rand(::Distributions.MvNormalCanon) currently extracts the Cholesky factor directly, without calling chol_lower:

https://github.com/JuliaStats/Distributions.jl/blob/50a712acb24cedda4291e26bdf52cd7f2096da89/src/multivariate/mvnormalcanon.jl#L179

@mschauer
Copy link
Member

This bit me. As far as I can see, PDSparseMat seems to be non-functional and should not be offered to the ecosystem.

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 a pull request may close this issue.

3 participants