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] Advance to Best Practices: Multivariate Gaussian Random Walk Notebook #195

Merged
merged 9 commits into from
Oct 2, 2021
Merged

[WIP] Advance to Best Practices: Multivariate Gaussian Random Walk Notebook #195

merged 9 commits into from
Oct 2, 2021

Conversation

ltoniazzi
Copy link
Member

@ltoniazzi ltoniazzi commented Jul 23, 2021

Description

Addresses issue #140 and aims to advance it to Best Practices

Best Practices

General updates: everything addressed.
ArviZ:

  • please doublecheck my usage of xarray.
  • Not sure how to use coords/dims effectively in this notebook.

Changes for discussion

  • Emphasized regression modelling.
  • Added docstrings and text between code blocks.
  • Added two ArviZ plots.
  • How to justify the Scaler (like with a reference about sampling improvement)?
  • When I run az.summary(trace).r_hat <1.03 one entry of a Cholesky matrix scores 1.04. Is that ok? (referring to this guide)

Cheers


Comment: I'll be available to work on this again on the 22nd of August.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@ltoniazzi ltoniazzi changed the title [WIP] Advance to Best Practives: Multivariate Gaussian Random Walk Notebook [WIP] Advance to Best Practices: Multivariate Gaussian Random Walk Notebook Jul 23, 2021
@MarcoGorelli MarcoGorelli self-requested a review July 23, 2021 08:11
@MarcoGorelli MarcoGorelli self-requested a review July 26, 2021 17:46
@@ -2,6 +2,7 @@
"cells": [
Copy link
Member

Choose a reason for hiding this comment

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

Line #43.            ppc = pm.sample_posterior_predictive(trace=trace)

I would directly add this to the inferencedata. I think the following should work.

trace.extend(az.from_pymc3(posterior_predictive=pm.sample_posterior_predictive(trace)))



Reply via ReviewNB

Copy link
Member Author

Choose a reason for hiding this comment

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

I did this and introduced coords. Everything seems to work but I get an odd message after the ppc sampling, namely the output of In [6] of the last commit says:

0, dim: steps, 300 =? 300
1, dim: y_, 3 =? 3

Copy link
Member

Choose a reason for hiding this comment

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

This is a forgotten debug print that made it into latest arviz release, it has already been fixed in main but the new release is still pending

Copy link
Member

@OriolAbril OriolAbril left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, this makes the notebook much more clear

@OriolAbril
Copy link
Member

@ltoniazzi can you rebase on main to get rtd to generate a preview of the notebook? I am quite sure the references will work except the doc one but it would be great to check they really do

@OriolAbril
Copy link
Member

Looks great so far and shows how myst "unlocking" sphinx references on notebook markdown is extremely useful 😄

@OriolAbril OriolAbril merged commit de167ab into pymc-devs:main Oct 2, 2021
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.

3 participants