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

Move to v4 for Rethinking 2 #194

Merged
merged 13 commits into from
Jun 3, 2022
Merged

Conversation

percevalve
Copy link

Move to v2 for Rethinking 2

Starting with Statistical Rethinking 2 as this is the one I know the best.

Was using the script from pymc-example (scripts/rerun.py)

Some of the file got transformed automatically:

  • Rethinking_2/Chp_02.ipynb
  • Rethinking_2/Chp_03.ipynb
  • Rethinking_2/Chp_10.ipynb
  • Rethinking_2/End_of_chapter_problems/Chapter_2.ipynb
  • Rethinking_2/End_of_chapter_problems/Chapter_11.ipynb

This PR include #152 .

For Rethinking_2/Chp_04.ipynb:

  • Using the default return_inferencedata=True for the pm.sample so using the az.InferenceData for the the trace variables.
  • Depending on usage, I changed the trace['var'] to trace.posterior('var']when possible or when it was expecting a array with all the sample in one array (rather than one array per chain), I used az.extract_dataset

Other change needed was when a Panda Series was used to define a model, this was triggering a Elementwise error, adding .values turned out to be enough.

As the traces are az.InferenceData, calculating the mean returns an array (I meant an Xarray), so added .item(0) to keep the compatibility with the formula as they exist, not sure this is the most elegant solution.

Same thing for the size of the posterior distribution, you cannot just do len(trace['var'], so I used .sizes["sample"], again not sure this is the most elegant solution, but it works.

For the #152 :

  • Implemented the comment (only thing the comment was missing is that the first step is index 0)
  • move to sns.kdeplot following seaborn depreciation warning
  • change parameter bw to bw_method as per seaborn depreciation warning.
  • change the index in the initial Simulation of field trip, as it was only doing 15 steps (index 0 is when no step have been done as far as I understood it).

@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 Jun 2, 2022

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2022-06-02T16:59:54Z
----------------------------------------------------------------

This need to fixed https://github.com/pymc-devs/pymc/issues/5443

see how you are overestimating the standard deviation


@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2022-06-02T16:59:55Z
----------------------------------------------------------------

Line #2.    sns.kdeplot(x[4, :].flatten(), bw_method=0.01, ax=ax[0])

alternatively, we can use az.plot_kde


@percevalve
Copy link
Author

Issue pymc-devs/pymc#5443 seems to be over my pay grade, going to exclude Rethinking_2/Chp_02.ipynb from this PR.

Yes indeed, using az.plot_kde makes more sense, and removes a dependency with seaborn.

@aloctavodia
Copy link
Member

Issue pymc-devs/pymc#5443 seems to be over my pay grade, going to exclude Rethinking_2/Chp_02.ipynb from this PR.

Yes indeed, using az.plot_kde makes more sense, and removes a dependency with seaborn.

Sorry for not being clear: This is the change needed in Rethinking_2/Chp_02.ipynb

data = np.repeat((0, 1), (3, 6))
with pm.Model() as m:
    p = pm.Uniform("p", 0, 1)  # uniform priors
    w = pm.Binomial("w", n=len(data), p=p, observed=data.sum())  # binomial likelihood
    mean_q = pm.find_MAP()

    p_value = m.rvs_to_values[p]
    p_value.tag.transform = None
    p_value.name = p.name

    std_q = ((1 / pm.find_hessian(mean_q, vars=[p])) ** 0.5)[0]

    # display summary of quadratic approximation
    print("  Mean, Standard deviation\np {:.2}, {:.2}".format(mean_q["p"], std_q[0]))

@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2022-06-03T14:57:08Z
----------------------------------------------------------------

This cell also needs to be fixed, similar to the previous one.


@aloctavodia aloctavodia merged commit 51c06ba into pymc-devs:main Jun 3, 2022
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