Skip to content

Implement random method for LKJCorr#2443

Merged
junpenglao merged 13 commits intopymc-devs:masterfrom
junpenglao:lkj_random
Aug 3, 2017
Merged

Implement random method for LKJCorr#2443
junpenglao merged 13 commits intopymc-devs:masterfrom
junpenglao:lkj_random

Conversation

@junpenglao
Copy link
Member

@junpenglao junpenglao commented Jul 25, 2017

using the algorithm in LKJ 2009(vine method based on a C-vine)

  • implement test.

using the algorithm in LKJ 2009(vine method based on a C-vine)
P = np.zeros((n, n)) # partial correlations
r_triu = []

for k in range(n-1):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to vectorize it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can not see an easy way to do it, the step below converting the partial correlation to raw correlation is especially tricky.

@junpenglao
Copy link
Member Author

Any suggestion of implementing a test? I am stuck. @ferrine @aseyboldt

@ferrine
Copy link
Member

ferrine commented Jul 31, 2017

What about comparing kde with theoretical density?

for k in range(n-1):
beta -= 1/2
for i in range(k+1, n):
P[k, i] = stats.beta.rvs(a=eta, b=eta) # sampling from beta
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd better create these samples beforehand

@twiecki
Copy link
Member

twiecki commented Jul 31, 2017

Or you could run a KS-test between true and sampled density.

@junpenglao
Copy link
Member Author

I see - is there any similar test I can reference to in our test? @ferrine @twiecki

@aseyboldt
Copy link
Member

@junpenglao junpenglao changed the title [WIP] implement random method for LKJCorr Implement random method for LKJCorr Aug 1, 2017
@junpenglao
Copy link
Member Author

Thanks everyone! I think I finally figure it out ;-)

for k, i in zip(triu_ind[0], triu_ind[1]):
p = P[k, i]
for l in range(k-1, -1, -1): # convert partial correlation to raw correlation
p = p * np.sqrt((1-P[l, i]**2)*(1-P[l, k]**2)) + P[l, i]*P[l, k]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pep8 wants white spaces around math operators

samples = generate_samples(stats.beta.rvs, eta, eta,
dist_shape=self.shape,
size=size)
samples = (samples-0.5)*2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

white-spaces

for k, i in zip(triu_ind[0], triu_ind[1]):
p = P[k, i]
for l in range(k-1, -1, -1): # convert partial correlation to raw correlation
p = p * np.sqrt((1 - P[l, i]**2) *
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what i am doing here is slow in a for loop, should I change it to reduce @ColCarroll?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For stuff like this we could also consider to use numba if it is available. (check if it is available and create a no-op replacement if it is not).

self.tri_index[np.triu_indices(n, k=1)] = np.arange(shape)
self.tri_index[np.triu_indices(n, k=1)[::-1]] = np.arange(shape)

def _random(self, n, eta, size=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be static I suppose

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like size argument is ignored

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. I ignored the size as the _random method here can only generate 1 slide of the random matrix.

Copy link
Member Author

@junpenglao junpenglao Aug 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion - I manage to use the size and vectorized the _random method.

@junpenglao
Copy link
Member Author

junpenglao commented Aug 2, 2017

OK the implementation should be correct as the distribution of the matrix elements match beta() distribution. However I am getting none positive-definite matrix when the dimension n>3:

import numpy as np
from scipy import stats

# cherry picked LKJ random function
def lkj_random(n, eta, size=None):
    beta0 = eta - 1 + n/2
    shape = n * (n-1) // 2
    triu_ind = np.triu_indices(n, 1)
    beta = np.array([beta0 - k/2 for k in triu_ind[0]])
    # partial correlations sampled from beta dist.
    P = np.ones((n, n) + (size,))
    P[triu_ind] = stats.beta.rvs(a=beta, b=beta, size=(size,) + (shape,)).T
    # scale partial correlation matrix to [-1, 1]
    P = (P - .5) * 2
    
    for k, i in zip(triu_ind[0], triu_ind[1]):
        p = P[k, i]
        for l in range(k-1, -1, -1):  # convert partial correlation to raw correlation
            p = p * np.sqrt((1 - P[l, i]**2) *
                            (1 - P[l, k]**2)) + P[l, i] * P[l, k]
        P[k, i] = p
        P[i, k] = p

    return np.transpose(P, (2, 0 ,1))

def is_pos_def(A):
    if np.array_equal(A, A.T):
        try:
            np.linalg.cholesky(A)
            return 1
        except np.linalg.linalg.LinAlgError:
            return 0
    else:
        return 0

P = lkj_random(4, 1., 1000)
k=0
for i, p in enumerate(P):
    k+=is_pos_def(p)
print(k)

Thoughts? @aseyboldt

@aseyboldt
Copy link
Member

@junpenglao I haven't look at it in detail, but should there really be that many 1. in the partial correlations P? Maybe something like P = np.ones((n, n) + (size,)) / 2?

@junpenglao
Copy link
Member Author

junpenglao commented Aug 2, 2017

@aseyboldt at the end only the diagonal is 1. Currently LKJ random returns the upper triangular elements (same as the distribution) but I was just doing some test in the code above.

@aseyboldt
Copy link
Member

Sorry, you are right.
The partial correlations are just a scaled version of the precision matrix, right? So maybe you could use that to check the result?

@junpenglao
Copy link
Member Author

junpenglao commented Aug 2, 2017

@aseyboldt not sure I understand what you mean.

I also compare with a Julia implementation, both have the same output. So it might be a numerical stability issue.

[EDIT], trying to figure out whether I have the same output as Julia or Stan.

@aseyboldt
Copy link
Member

hm. But shouldn't A_{ij}/np.sqrt(A_ii * A_jj) equal the original partial correlations, were A = \sigma^{-1}? It isn't. And sometimes the eigenvalues are definitely negative, much more so than could be explained by some simple rounding.

@aseyboldt
Copy link
Member

I'm using this to compute the partial correlations:

tau = linalg.inv(P[0])

partial = tau.copy()
partial /= np.sqrt(np.diag(tau))[None, :]
partial /= np.sqrt(np.diag(tau))[:, None]

(And just added a print statement in the function to get the original values)

@junpenglao
Copy link
Member Author

junpenglao commented Aug 2, 2017

I will also compare with the implementation in R from @rmcelreath https://github.com/rmcelreath/rethinking/blob/master/R/distributions.r#L165-L184, I cannot really figure it out in Stan and Julia

@aseyboldt
Copy link
Member

Sorry, my last comment was wrong, I looked at the wrong array. I just deleted it.

generated Corr matrix is now positive definite.
@junpenglao
Copy link
Member Author

Turns out the R implementation from @rmcelreath is the most stable - test pass locally and samples are mostly positive definite (failed sometimes when n is large and eta<<1, but much better than the previous implementation nonetheless).

@junpenglao
Copy link
Member Author

@aseyboldt The current random method could potentially be extended for generating LKJCholskyCov as it produces a triangular matrix as an intermedia step.

@junpenglao junpenglao merged commit f845575 into pymc-devs:master Aug 3, 2017
@junpenglao junpenglao deleted the lkj_random branch August 3, 2017 13:48
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 this pull request may close these issues.

4 participants