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

[WIP] Ch6 #40

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

[WIP] Ch6 #40

wants to merge 6 commits into from

Conversation

canyon289
Copy link
Collaborator

No description provided.

@AlexAndorra
Copy link

Thank you for this hard and useful work @canyon289 and @aloctavodia !
Ravin, I can help you if you need? Particularly on chapters 6 (Mixture Models) and 7 (Gaussian Processes). I just finished the exercises on these models for Richard McElreath's book, so it's still fresh in my head 😉
Of course, I can also assist on other chapters, but looks like they are quite ready.

@canyon289
Copy link
Collaborator Author

Hey @AlexAndorra would you be open to reviewing as I finish the exercises? Notebooks are hard to work on in parallel but an extra set of eyes would be helpful!

@AlexAndorra
Copy link

Sure, would be happy to!
Which chapters are the most pressing for review?

@canyon289
Copy link
Collaborator Author

Any of the ones in PR really. Just a heads up though. Osvaldo and I are reeeaaalllllyyyy slow on this. Like we've taken months, and sometimes take multiple weeks to respond to each other. If were slow with you too please don't be frustrated :)

@AlexAndorra
Copy link

Ha ha I totally understand! I'm doing that on my spare time too, which is quite cyclical (but, sadly, rather hard to predict, even with a Bayesian model...).
Will take a look at the PRs ASAP (may have to re-read some chapters first 😝).

@canyon289
Copy link
Collaborator Author

No worries. Thank you for your time. We really appreciate your efforts :)

@AlexAndorra
Copy link

AlexAndorra commented Jul 24, 2019

Going through the exercises, a first thought is that it would be useful for readers to have the text of the questions, that are in the book, in addition to the answers.
If @aloctavodia is ok with that, I can do a PR to add it to the 4 notebooks already merged. Not sure I'm able to do it on the NBs currently in review though (maybe with the Review NB app?)...

@canyon289
Copy link
Collaborator Author

If @aloctavodia agrees I can add them into the ones in review to make things easier

@AlexAndorra
Copy link

That would be great!
Also, allow the Review NB App access to this repo would make the review process easier I think. But I guess we have to wait for our dear Oslvado to do that ;)

@aloctavodia
Copy link
Owner

Happy to see you both working together, I agree with both of you! :-)
I am slowly transitioning from vacation to going back to work :-)

@AlexAndorra
Copy link

AlexAndorra commented Jul 26, 2019 via email

@canyon289
Copy link
Collaborator Author

So either @aloctavodia or @AlexAndorra I could use help! On this section for question 4 to 6 I'm getting a ton of divergences and not really sure what to do about it. Tried increasing tuning and number of samples but no luck :(

@AlexAndorra
Copy link

Thank you Ravin! I'll try to take a look this week or the next ;)

@AlexAndorra
Copy link

@aloctavodia do you think you could set up the Review NB App on this repo? I think it would make the review process easier ;) Thank you!

@AlexAndorra
Copy link

So I managed to take a look at your NB @canyon289 🎉 Here are my (deep) thoughts (sorry it's not very convenient, but without the Review NB app I don't see any other way):

  • In general, it looks like all the models needed more informative priors. Iteratively and with some prior predictive checks, I think I found some priors (particularly on the Dirichlet) that allow the models to sample quite smoothly (but it's very probable it's not the best solution and that @aloctavodia will come up with something better)

  • Exercise 1: there were a label-switching problem and divergences in the three models (a few for 2 and 3 clusters, a lot for 4 clusters). Adopting more regularizing priors and ordering the means solved it for 2 and 3 clusters, but the 4-cluster one still has a few divergences, despite my numerous attemtps. I suspect it is overparametrized, as it tries to find 4 clusters within 3-cluster data. Here is the modified model:

with pm.Model() as two_components:
        p = pm.Dirichlet("p", a=np.array([10.]*cluster))

        # Each mean of the mixture data has its own estimate of a mean with a fixed SD in this case
        means = pm.Normal("means", mu=np.linspace(vals.min(), vals.max(), cluster), 
                          sd=10., shape=cluster, transform=pm.distributions.transforms.ordered)
        
        # Estimate of the standard deviation of the whole population
        sd = pm.HalfCauchy("sd", 1.)
        y = pm.NormalMixture("y", w=p, mu=means, sd=sd, observed=vals)
        
        trace = pm.sample(1000, tune=5000, cores=2, random_seed=123)
  • Exercise 2: I just wanted to note that az.compare(traces) throws me a ValueError: could not broadcast input array from shape (300,1) into shape (300). Is it the same for you? Nothing other than that. The WAIC-LOO comparison still (rightly) ranks the 3-cluster model on top, and the traces look a lot healthier with the new model.

  • Execise 4: This one is a real head-scratcher... I tried ordering the means and changing the gamma prior but nothing worked. I think the model is misspecified but I really don't know where...

  • Exercise 5: Here, adopting regularizing priors and ordering the means made the model sample smoothly. The traces look better, although they are not perfect - I think the model has trouble distinguishing two species that have very similar probability. Plus, the third species' probability seems to be close to 0. Here is the model:

with pm.Model() as model_mg:
    
    p = pm.Dirichlet('p', a=np.array([5.]*clusters))
    
    means = pm.Normal('means', mu=np.linspace(sepal_length.min(), sepal_length.max(), clusters), 
                      sd=10., shape=clusters, transform=pm.distributions.transforms.ordered)
    
    sd = pm.HalfNormal('sd', sd=10.)
    
    y = pm.NormalMixture('y', w=p, mu=means, sd=sd, observed=sepal_length)
    
    sepal_trace = pm.sample(1000, tune=6000, cores=2, nuts_kwargs={"target_accept": 0.9})
  • Exercise 6: Same thing as above, with the same regularizing priors, but I have a question here. If I understood correctly, this model clusters the species based on sepal_width and sepal_length independently (we could actually do two separate models and sample from them independently). Is there a way to use both features at the same time in the same model - i.e cluster the species based both on sepal_width and sepal_length?

@canyon289
Copy link
Collaborator Author

@AlexAndorra Thanks for the feedback here and on ch7

For question 2: I'm using the ArviZ version in the environment.yml file. At one point there was code introduced in ArviZ that created the exception you're referring to but I think its gone now.

For Exercise 6. I'm actually not sure. I thought the way I specified the model is how MultiObserved works in ArviZ. I'll ask on the PyMC discourse to be sure.

As for the rest I appreciate you going through them! I'll implement your suggestions this weekend.

PyMCheers!

@AlexAndorra
Copy link

You're welcome Ravin ;)

FYI, I'm using Arivz's latest version (0.4.1), which means a novelty broke this specific code... Do I need to open an issue on Arivz's repo?

For exercise 6, my knowledge stops there on the matter, but I'm curious about the answer. I'll follow your question on Discourse!

@canyon289
Copy link
Collaborator Author

Fixed exercise 2 per Alex's notes. Thank you!

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.

None yet

3 participants