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

Replacing PyMC3 plots w/ Arviz plots & sigma Param change #16

Closed

Conversation

CloudChaoszero
Copy link
Contributor

@CloudChaoszero CloudChaoszero commented Jan 4, 2021

Description

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

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

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@CloudChaoszero CloudChaoszero changed the title [WIP] Start Replacing PyMC3 plots with arviz plots in examples [WIP] Replacing PyMC3 plots w/ Arviz plots & sigma Param change Jan 5, 2021
@AlexAndorra AlexAndorra linked an issue Jan 17, 2021 that may be closed by this pull request
@CloudChaoszero CloudChaoszero changed the title [WIP] Replacing PyMC3 plots w/ Arviz plots & sigma Param change Replacing PyMC3 plots w/ Arviz plots & sigma Param change Jan 18, 2021
@CloudChaoszero CloudChaoszero marked this pull request as ready for review January 18, 2021 10:19
* Related to issue pymc-devs/pymc#4371

🎨 Replace Pymc3 plots w/arviz plots and sigma param change

🎨 Update plots to arviz dependency instead of pymc3 dependency

🎨 Update plots to arviz dependency instead of pymc3 dependency

🎉 Finish residual replacement for Pymc3 plots to arviz & sigma param change
@twiecki
Copy link
Member

twiecki commented Jan 18, 2021

Woah, @CloudChaoszero is this done / ready for review? Did you do it manually? Did you re-execute the NBs?

@CloudChaoszero
Copy link
Contributor Author

CloudChaoszero commented Jan 18, 2021

@CloudChaoszero is this done / ready for review? Did you do it manually? Did you re-execute the NBs?

Yeah, @twiecki I would say this is done! haha (I was watching some tv shows while working on this, over time). This was done manually, and I re-executed most books***.

I do have some notes:

Notes

  1. I'm confident I replaced all Pymc3 plots & diagnostics, regarding Remove Plots & Diagnostics from PyMC Code Base pymc#4371 (except for plot_posterior_predictive_glm, that stays)
  2. ***There were some files with hiccups/errors, so I did not re-run them. (Errors & issues like what are listed here https://github.com/pymc-devs/pymc3/issues/3959) However, I did replace functions like pm.traceplot w/ az.plot_trace, etc
  3. Added a couple warnings.ignore() because the notebooks that ran had my local path printed on the Warnings (e.g. Warning: C://User/Username/....ipynb etc)

@twiecki
Copy link
Member

twiecki commented Jan 18, 2021

Amazing and much appreciated!

@AlexAndorra
Copy link
Collaborator

Thanks a lot for this @CloudChaoszero 🤩
I have one request though: the PR is currently huge, so could you chunk it off in several different PRs, each having, let's say 10 notebooks, and then a last one with the .py files?
That way, it'll be much easier and faster to review for us, and it'll be easier for you to update the files according to our comments. How does that sound?

@CloudChaoszero
Copy link
Contributor Author

CloudChaoszero commented Jan 19, 2021

I have one request though: the PR is currently huge, so could you chunk it off in several different PRs, each having, let's say 10 notebooks, and then a last one with the .py files?
That way, it'll be much easier and faster to review for us, and it'll be easier for you to update the files according to our comments. How does that sound?

@AlexAndorra That works! haha

@MarcoGorelli
Copy link
Contributor

Thanks @CloudChaoszero - OK, shall we close this PR then in favour of the others? (I'll take a look at the during the coming week)

@AlexAndorra
Copy link
Collaborator

Yeah let's do that. Thanks @CloudChaoszero

@CloudChaoszero CloudChaoszero deleted the replace-pymc3-arviz-plots branch April 5, 2021 01:49
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.

Use sigma instead of sd
4 participants