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

multivariate mixture random method is broken #3270

Closed
junpenglao opened this Issue Nov 24, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@junpenglao
Copy link
Member

junpenglao commented Nov 24, 2018

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

The problem is the weight random is incorrect:

w_samples = generate_samples(random_choice,
p=w,
broadcast_shape=w.shape[:-1] or (1,),
dist_shape=distshape,
size=size).squeeze()

@junpenglao

This comment has been minimized.

Copy link
Member

junpenglao commented Nov 24, 2018

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

This comment has been minimized.

Copy link
Contributor

lucianopaz commented Dec 5, 2018

@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

This comment has been minimized.

Copy link
Member

junpenglao commented Dec 5, 2018

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/pymc3 that referenced this issue Dec 7, 2018

@twiecki twiecki closed this in #3293 Jan 3, 2019

twiecki added a commit that referenced this issue Jan 3, 2019

Various fixes to random sampling
* 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