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

Update Ch2_MorePyMC_PyMC_current.ipynb print initial values, in accordance with latest pymc version #558

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sebastianduchene
Copy link

model.initial_values.items is deprecated. I updated it to model.rvs_to_initial_values to print variables and initial values correctly.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@twiecki
Copy link
Contributor

twiecki commented Apr 19, 2023

Thanks @sebastianduchene! Any idea why the results changed so strongly?

@sebastianduchene
Copy link
Author

sebastianduchene commented Apr 20, 2023

Hey @twiecki, That's a good point. I thought it was Monte Carlo error for a second, but then I realised that it was something much simpler. The notebook has a random seed but it isn't really working, meaning that the simulated data sets can look quite different. For example, in A Simple Case the data simulated from the Bernoulli have 91 'successes', but the expected number is 0.05 * 1500 = 75, and when I ran it in my machine I got 78, which is not too different from the expected and therefore the true value of p_A falls well within the posterior.
Similarly, in A and B Together, the data are simulated from a Bernoulli, and probably are a bit different to those from the model, leading to different posterior distros for p_a, p_B and delta.

In contrast, in Example: Challenger Space Shuttle Disaster, everyone gets the same posterior because the data are fixed.

I tried doing :
np.random.seed(RANDOM_SEED)

instead of:
rng = np.random.default_rng(RANDOM_SEED)

which seems to work. I'll give this a shot tomorrow and push it if it works well.

@twiecki
Copy link
Contributor

twiecki commented Apr 21, 2023

Actually the second approach is the recommended one, but we then need to pass the rng everywhere, which is a bit clunky too. So maybe try the first approach.

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.

2 participants