Skip to content

Update notebook Dirichlet mixture of multinomials to PyMC 5 #581

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Oct 5, 2023
Merged

Update notebook Dirichlet mixture of multinomials to PyMC 5 #581

merged 9 commits into from
Oct 5, 2023

Conversation

erik-werner
Copy link
Contributor

@erik-werner erik-werner commented Oct 5, 2023 •
edited by github-actions bot
Loading

Issue #100

📚 Documentation preview 📚: https://pymc-examples--581.org.readthedocs.build/en/581/

@erik-werner
Update notebook to pymc v5
b0dffc7 
Also remove unused import
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@review-notebook-app
Copy link

review-notebook-app bot commented Oct 5, 2023 •
edited
Loading

View / edit / reply to this conversation on ReviewNB

ricardoV94 commented on 2023-10-05T10:56:08Z
----------------------------------------------------------------

We should pass rng to .rvs(random_state=rng so the data is fixed.


erik-werner commented on 2023-10-05T11:15:08Z
----------------------------------------------------------------

Yep, I had actually just fixed this when I saw your comment :)

I'm working on some more updates, so it might be good to hold off on reviewing the rest of the notebook for now.

@erik-werner erik-werner changed the title Update notebook Dirichlet mixture of multinomials to PyMC 5 [WIP] Update notebook Dirichlet mixture of multinomials to PyMC 5 Oct 5, 2023
@erik-werner Review Notebook App
Copy link
Contributor Author

Yep, I had actually just fixed this when I saw your comment :)

I'm working on some more updates, so it might be good to hold off on reviewing the rest of the notebook for now.

View entire conversation on ReviewNB

@erik-werner
Update notebook to match new simulated data
32dcc0a 
Since the simulated data has changed, the numerical issues we faced are slightly different.
The notebook has been modified to reflect this.

- No need to do metropolis sampling for the multinomial model
- Worse numerical issues for the explicit DM model. Addressed by increasing
target_accept and modify the discussion about poor rhat values
- Minor changes to match new data
- Also refer to species by name instead of index.
@erik-werner
Minor changes to notebook text
e05d4d4
@erik-werner
Change target_accept from 0.95 to 0.9
9f9d8b0 
This gets rid of most divergences, and gives much better r_hat statistics
@erik-werner
Add myself to author list
a74068d
@erik-werner
Minor change
69b4554
@erik-werner
Fix minor text mismatches after updates
e83d3ef
@erik-werner erik-werner changed the title [WIP] Update notebook Dirichlet mixture of multinomials to PyMC 5 Update notebook Dirichlet mixture of multinomials to PyMC 5 Oct 5, 2023
@erik-werner erik-werner marked this pull request as ready for review October 5, 2023 12:15
twiecki
twiecki requested changes Oct 5, 2023
View reviewed changes
Copy link
Member

@twiecki twiecki left a comment

Choose a reason for hiding this comment

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

pre-commit is failing.

@erik-werner
Copy link
Contributor Author

pre-commit is failing.

But the errors are unrelated to my changes:

black-jupyter.................................................................................Failed
- hook id: black-jupyter
- files were modified by this hook

reformatted examples/case_studies/nyc_bym.ipynb
reformatted examples/howto/model_builder.ipynb

All done! ✨ 🍰 ✨
2 files reformatted, 129 files left unchanged.

Shall I fix them as part of this pull request anyway? It's easily done. Or do you want me to open a new pull request where I fix these errors?

@twiecki
Copy link
Member

twiecki commented Oct 5, 2023

@erik-werner Ah, in that case you can:

  1. just don't do anything and we merge
  2. fix it here
  3. don't fix it here and do a separate PR

Which do you prefer?

@erik-werner
Copy link
Contributor Author

@twiecki I'll fix it here then!

@twiecki twiecki merged commit 0e74cef into pymc-devs:main Oct 5, 2023
@twiecki
Copy link
Member

twiecki commented Oct 5, 2023

Thanks @erik-werner, this is a great contribution!

@erik-werner erik-werner deleted the update_dirichlet_multinomial_notebook branch October 5, 2023 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@twiecki twiecki twiecki requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@erik-werner @twiecki