Skip to content

multivariate mixture random method is broken #3270

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

Closed
junpenglao opened this issue Nov 24, 2018 · 3 comments · Fixed by #3293
Closed

multivariate mixture random method is broken #3270

junpenglao opened this issue Nov 24, 2018 · 3 comments · Fixed by #3293
Assignees

Comments

@junpenglao
Copy link
Member

https://discourse.pymc.io/t/shape-mismatch-error-in-sample-ppc/2270

The problem is the weight random is incorrect:
https://github.com/pymc-devs/pymc3/blob/6d07591962a6c135640a3c31903eba66b34e71d8/pymc3/distributions/mixture.py#L160-L164

@junpenglao
Copy link
Member Author

OK, the problem is more serious...
Currently, we are generating categorical latent index from weight and component random from comp_dist, if the weight is a vector, we index the component random: comp_random[..., weight_random], but it is incorrect as we should generate new sample for comp_random for each slice...

dist = pm.NormalMixture.dist(w=np.array([.5, .25, .25]), 
                             mu=np.array([-1., 1., 0]), 
                             sd=1, 
                             shape=10)
dist.random()

array([ 0.43626031,  1.59448573,  0.43626031, -0.45223364, -0.45223364,
       -0.45223364,  0.43626031, -0.45223364,  0.43626031,  1.59448573])

As you can see above, we have identical value when the latent index is the same, but it shouldnt be the case...

@lucianopaz
Copy link
Contributor

@junpenglao, I was trying to fix this problem today. I managed to get the broadcasting working for sampling from the posterior predictive. To make sure that the broadcasting was working well, I also wanted to try to sample from the prior predictive but got stumped by a separate problem which maybe is not important for this issue. The LKJCholeskyCov distribution has no random method. Does it make sense to try to implement it from LKJCorr.random? I was trying to do that and got into so much more shape problems

@junpenglao
Copy link
Member Author

Yes it make sense if you are up for the challenge ;-) you need to generate from LKJCorr and the diagnoal dist and than combine them together.

lucianopaz added a commit to lucianopaz/pymc that referenced this issue Dec 7, 2018
twiecki pushed a commit that referenced this issue Jan 3, 2019
* Partial fix. Got stumped with LKJCholeskyCov not having a random method

* Fixed the basic defects. Still must do new tests.

* Added test for sampling prior and posterior predictives from a mixture based on issue #3270

* Fixed bugs and failed tests. Still must write a test for LKJCholeskyCov.random method.

* Fixed failed test

* Added tests for DrawValuesContext and also for the context blocker that was introduced in this PR.

* Changed six metaclass to metaclass keyword
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants