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

Fix support of LKJCorr #1863

Merged
merged 2 commits into from
Mar 10, 2017
Merged

Fix support of LKJCorr #1863

merged 2 commits into from
Mar 10, 2017

Conversation

aseyboldt
Copy link
Member

LKJCorr did not use an interval transformation to fix the support of the entries of the corr matrix to [-1, 1]. Because of this advi did not work at all and nuts reported diverging transitions in almost every sample:

with pm.Model() as model:
    corr = pm.LKJCorr('corr', n=1, p=2)
    trace = pm.sample(2000, tune=1000, init=None)
   trace['diverging'].mean()

gives 0.998.

This should fix #1852.

@twiecki
Copy link
Member

twiecki commented Mar 6, 2017

Great catch!

======================================================================
ERROR: test_lkj_0 (pymc3.tests.test_distributions.TestMatchesScipy)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/miniconda2/envs/testenv/lib/python2.7/site-packages/nose_parameterized/parameterized.py", line 365, in standalone_func
    return func(*(a + p.args), **p.kwargs)
  File "/home/travis/build/pymc-devs/pymc3/pymc3/tests/test_distributions.py", line 561, in test_lkj
    assert_almost_equal(model.fastlogp(pt), lp, decimal=6, err_msg=str(pt))
  File "/home/travis/build/pymc-devs/pymc3/pymc3/model.py", line 724, in __call__
    return self.f(**state)
  File "/home/travis/miniconda2/envs/testenv/lib/python2.7/site-packages/theano/compile/function_module.py", line 822, in __call__
    self[k] = arg
  File "/home/travis/miniconda2/envs/testenv/lib/python2.7/site-packages/theano/compile/function_module.py", line 512, in __setitem__
    self.value[item] = value
  File "/home/travis/miniconda2/envs/testenv/lib/python2.7/site-packages/theano/compile/function_module.py", line 463, in __setitem__
    (str(item), msg))
TypeError: Unknown input or state: lkj. The function has 1 named input (lkj_interval_).
-------------------- >> begin captured logging << --------------------
pymc3: DEBUG: Applied interval-transform to lkj and added transformed lkj_interval_ to model.
--------------------- >> end captured logging << ---------------------

@AustinRochford
Copy link
Member

I recently ran into this, thanks for catching it!

@fonnesbeck
Copy link
Member

fonnesbeck commented Mar 8, 2017

@aseyboldt Looks like the key to the dict on line 560 of test_distributions needs to be the transformed variable rather than the original.

@aseyboldt
Copy link
Member Author

aseyboldt commented Mar 10, 2017

I fixed it by disabling the transformation. Is there a better way that does not involve adding the log det manually in the tests?

@fonnesbeck
Copy link
Member

That's probably fine, given the intention of the test.

@twiecki twiecki merged commit f3028db into pymc-devs:master Mar 10, 2017
@aseyboldt aseyboldt deleted the fix-lkjcorr branch March 10, 2017 21:07
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.

NaN ELBO values with LKJcorr prior
4 participants