Replacing PyMC3 plots w/ Arviz plots & sigma Param change [Part 3]

[CloudChaoszero]

Description

The following is a large PR breakdown of PR #16.

Replace PyMC3 dependent plots with arviz plots in case studies & examples.

• The issue is a 3-part step for the following:
  • Initiative via Issue Remove Plots & Diagnostics from PyMC Code Base pymc#4371 (Leave open)

Replace parameter sd with sigma (e.g. some examples have pm.Normal(...sd=...)

• Closes issue Use sigma instead of sd #2

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[MarcoGorelli]

Hi @CloudChaoszero - it looks like the watermarks haven't changed, could you please run the notebooks top-to-bottom? It may be that some other parts need updating (I've often found that when updating old notebooks)

[CloudChaoszero]

@MarcoGorelli Pardon about that. haha I went ahead and did the full runs, and squashed the commits.

[CloudChaoszero]

Small note

There were a couple of errors I encountered like:

examples/diagnostics_and_criticism/Diagnosing_biased_Inference_with_Divergences.ipynb:

• divergent_point had a dimension/indice error in section

ii = 5

tau_log_d = divergent_point["tau_log__"]
theta0_d = divergent_point["theta"][:, ii]
Ndiv_recorded = len(tau_log_d)

So I removed the [:, ii], since it's a singular dimension array, and not 2-dim.

examples/diagnostics_and_criticism/Bayes_factor.ipynb:

• parallel=True caused an error. It is recorded as a defect, based on this issue

        trace = pm.sample_smc(1000, random_seed=42, parallel=True)

[MarcoGorelli]

Thanks @CloudChaoszero for updating!

TBH I'm really not keen on adding

warnings.filterwarnings("ignore", category=FutureWarning)
warnings.filterwarnings("ignore", category=UserWarning)

. If there's a specific warning that needs disabling in one cell, then OK, but disabling everywhere doesn't strike me as a great practice. CC @AlexAndorra any thoughts on disabling warnings in notebooks?

[AlexAndorra]

If there's a specific warning that needs disabling in one cell, then OK, but disabling everywhere doesn't strike me as a great practice. CC @AlexAndorra any thoughts on disabling warnings in notebooks?

I agree with @MarcoGorelli : we already know many people don't pay enough attention to warnings, so if we also disable them in notebooks, that makes our job more difficult. On the contrary, displaying them in notebooks is a great way to make people more aware of important warnings -- for instance, users should really know that we're gonna switch to InferenceData by default at one point, and make breaking changes in the next major version, etc.

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

OriolAbril commented on 2021-02-07T21:02:50Z
----------------------------------------------------------------

The magenta lines in the right subplot are gone somehow

CloudChaoszero commented on 2021-02-08T06:17:38Z
----------------------------------------------------------------

Yeah, I'm not to sure why, either.

* I was able to update the other recommendations though, outside of this comment.

[OriolAbril]

I suggest using:

ppc_w = pm.sample_posterior_predictive_w(
    traces,
    models=[model_0, model_1, model_2],
    weights=comp.weight.sort_index(ascending=True),
    progressbar=True,
)

Reply via ReviewNB

[OriolAbril]

and here:

ppc_2 = pm.sample_posterior_predictive(trace_2, model=model_2, progressbar=False)

Reply via ReviewNB

[OriolAbril]

Not sure why there is a mean after the call to hdi, I would suggest doing:

hpd_w = az.hdi(ppc_w["kcal"].flatten())

instead. If I'm not wrong, between this and the changes to sample_... there won't be a need to ignore warnings anymore everything will have been solved and will follow best practices

Reply via ReviewNB

[OriolAbril]

There should be no need to ignore warnings in this notebook, ArviZ does not warn about the change in default anymore.

Reply via ReviewNB

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

OriolAbril commented on 2021-02-07T21:02:53Z
----------------------------------------------------------------

no need to ignore warnings here either

[OriolAbril]

This plot is actually az.plot_energy the cell can be reduced to a single line (could be done in a follow up PR)

Reply via ReviewNB

[CloudChaoszero]

If there's a specific warning that needs disabling in one cell, then OK, but disabling everywhere doesn't strike me as a great practice. CC @AlexAndorra any thoughts on disabling warnings in notebooks?

I agree with @MarcoGorelli : we already know many people don't pay enough attention to warnings, so if we also disable them in notebooks, that makes our job more difficult. On the contrary, displaying them in notebooks is a great way to make people more aware of important warnings -- for instance, users should really know that we're gonna switch to InferenceData by default at one point, and make breaking changes in the next major version, etc.

Yeah, I just did it to not show my local path when the Warnings come up. lol

I can remove them going forward, if you all are okay with that.

[CloudChaoszero]

Yeah, I'm not to sure why, either.

* I was able to update the other recommendations though, outside of this comment.

---

View entire conversation on ReviewNB

[OriolAbril]

We should open an issue about the wrong plot parallel in the divergences notebook so we don't forget to fix eventually everything else looks good