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 [Part 4] #24

Merged

Conversation

CloudChaoszero
Copy link
Contributor

Description

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

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

@review-notebook-app
Copy link

review-notebook-app bot commented Feb 7, 2021

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2021-02-07T21:33:37Z
----------------------------------------------------------------

plot is missing


CloudChaoszero commented on 2021-02-14T08:51:31Z
----------------------------------------------------------------

There appears to be a number of errors in this notebook. I display some in this (personal) PR.

One error in particular that I first come across is

GLM-model-selection KeyError: 'var names: "['sd_log__'] are not present" in dataset'

It's noted in this raised issue

CloudChaoszero commented on 2021-02-14T08:52:58Z
----------------------------------------------------------------

Hmm, should we just revert this Notebook to the upstream/main state, thoughts?

michaelosthege commented on 2021-03-08T11:29:30Z
----------------------------------------------------------------

Try calling pyplot.show().

The interactive plots are always a bit dangerous, because they often get lost when saving the notebook. Maybe you can find out how to tell nbconvert to save the widget state and then run the notebook from the command line?

This does NOT save the widget state: jupyter nbconvert --ExecutePreprocessor.kernel_name="python3" --ExecutePreprocessor.timeout=14000 --execute --inplace {}

michaelosthege commented on 2021-03-08T11:35:28Z
----------------------------------------------------------------

The {} is where the notebook filename goes

CloudChaoszero commented on 2021-03-14T05:05:12Z
----------------------------------------------------------------

@michaelosthege thanks for the response! I'm somewhat confused. I get this error

var names: "['sd_log__']

I created a gist of showing the example. :/

https://gist.github.com/CloudChaoszero/09b566b857190cb04391c615dc08c129

michaelosthege commented on 2021-03-14T11:16:03Z
----------------------------------------------------------------

The key belongs to the transform of sd, but it looks like sd is sampled untransformed. I'm not familiar with this notebook and find the pandas stuff happening above quite confusing. @twiecki do you know if this model selection can be simplified with newer ArviZ features? Having to work with straces directly is not something we should need to teach 😬

Multiprocess sampling (2 chains in 2 jobs) 
NUTS: [sd, np.power(x, 5), np.power(x, 4), np.power(x, 3), np.power(x, 2), x, Intercept] 

@review-notebook-app
Copy link

review-notebook-app bot commented Feb 7, 2021

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2021-02-07T21:33:38Z
----------------------------------------------------------------

plot is missing


CloudChaoszero commented on 2021-02-14T08:52:26Z
----------------------------------------------------------------

For errors in this notebook, see the other mentioned comment about issues in this notebook

@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2021-02-07T21:33:38Z
----------------------------------------------------------------

plot is missing


@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2021-02-07T21:33:39Z
----------------------------------------------------------------

table is missing


@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2021-02-07T21:33:40Z
----------------------------------------------------------------

plot is missing


@review-notebook-app
Copy link

review-notebook-app bot commented Feb 7, 2021

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2021-02-07T21:33:41Z
----------------------------------------------------------------

not sure what annotate now is but this is an error


CloudChaoszero commented on 2021-02-14T08:30:59Z
----------------------------------------------------------------

Looks like the library deprecated the annotate function, [seen in this Seaborn Issue](https://github.com/mwaskom/seaborn/issues/2306) :/

@review-notebook-app
Copy link

review-notebook-app bot commented Feb 7, 2021

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2021-02-07T21:33:41Z
----------------------------------------------------------------

colors here are mixed up in the 2D plot, should match marginals


@review-notebook-app
Copy link

review-notebook-app bot commented Feb 7, 2021

View / edit / reply to this conversation on ReviewNB

twiecki commented on 2021-02-07T21:33:42Z
----------------------------------------------------------------

ugh, damn, this convergence is really bad. maybe try with target_accept=0.99


CloudChaoszero commented on 2021-02-14T08:28:51Z
----------------------------------------------------------------

Sounds good. I added a target_accept = 0.99

…h sigma

⏪ Revert changes due to improper metadata updates

🎨 Fix divergence in last cells and outdated errors due to variou package changes
@CloudChaoszero CloudChaoszero force-pushed the replace-pymc3-arviz-plots_part4 branch from db1afed to 9662276 Compare February 14, 2021 08:28
Copy link
Contributor Author

Sounds good. I added a target_accept = 0.99


View entire conversation on ReviewNB

Copy link
Contributor Author

Looks like the library deprecated the annotate function, [seen in this Seaborn Issue](https://github.com/mwaskom/seaborn/issues/2306) :/


View entire conversation on ReviewNB

Copy link
Contributor Author

There appears to be a number of errors in this notebook. I display some in this (personal) PR.

One error in particular that I first come across is

GLM-model-selection KeyError: 'var names: "['sd_log__'] are not present" in dataset'

It's noted in this raised issue


View entire conversation on ReviewNB

Copy link
Contributor Author

For errors in this notebook, see the other mentioned comment about issues in this notebook


View entire conversation on ReviewNB

Copy link
Contributor Author

Hmm, should we just revert this Notebook to the upstream/main state, thoughts?


View entire conversation on ReviewNB

Copy link
Member

Try calling pyplot.show().

The interactive plots are always a bit dangerous, because they often get lost when saving the notebook. Maybe you can find out how to tell nbconvert to save the widget state and then run the notebook from the command line?

This does NOT save the widget state: jupyter nbconvert --ExecutePreprocessor.kernel_name="python3" --ExecutePreprocessor.timeout=14000 --execute --inplace {}


View entire conversation on ReviewNB

@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

michaelosthege commented on 2021-03-08T11:32:21Z
----------------------------------------------------------------

In most other notebooks we have the watermark at the end?


Copy link
Member

The {} is where the notebook filename goes


View entire conversation on ReviewNB

@OriolAbril
Copy link
Member

In most other notebooks we have the watermark at the end?

Yes, all notebooks are supposed to have the watermark at the end.

The interactive plots are always a bit dangerous, because they often get lost when saving the notebook

Agreed, however, I don't think there are interactive plots in the notebook

KeyError: 'var names: "['sd_log__'] are not present" in dataset'

This is because ArviZ does not store the transformed variables in the posterior group. You can either use the transform argument in the plotting functions (I think they all have it but not sure). Or manually add the transformed variable to the dataset. See one example here: arviz-devs/arviz#641

Copy link
Contributor Author

@michaelosthege thanks for the response! I'm somewhat confused. I get this error

var names: "['sd_log__']

I created a gist of showing the example. :/

https://gist.github.com/CloudChaoszero/09b566b857190cb04391c615dc08c129


View entire conversation on ReviewNB

Copy link
Member

The key belongs to the transform of sd, but it looks like sd is sampled untransformed. I'm not familiar with this notebook and find the pandas stuff happening above quite confusing. @twiecki do you know if this model selection can be simplified with newer ArviZ features? Having to work with straces directly is not something we should need to teach 😬

Multiprocess sampling (2 chains in 2 jobs) 
NUTS: [sd, np.power(x, 5), np.power(x, 4), np.power(x, 3), np.power(x, 2), x, Intercept] 

View entire conversation on ReviewNB

@OriolAbril
Copy link
Member

Maybe we can create an issue as a remainder to fix this but for now go ahead and merge the PR?

@CloudChaoszero
Copy link
Contributor Author

Maybe we can create an issue as a remainder to fix this but for now go ahead and merge the PR?

Sounds good @OriolAbril ! I made this issue

@OriolAbril
Copy link
Member

I saw the issue, thanks! I don't see the comments of this PR on the issue yet, especially the ones about the glm comparison one, can you add them?

@CloudChaoszero
Copy link
Contributor Author

I saw the issue, thanks! I don't see the comments of this PR on the issue yet, especially the ones about the glm comparison one, can you add them?

Sounds good. @OriolAbril ! I added it at the last section, of the original description, and linked one of this PR's comments

@OriolAbril OriolAbril merged commit 885fb46 into pymc-devs:main Mar 21, 2021
@CloudChaoszero CloudChaoszero deleted the replace-pymc3-arviz-plots_part4 branch April 5, 2021 01:53
twiecki pushed a commit that referenced this pull request Jan 17, 2023
* change  workflows directory

* change  workflows directory

* use last version of pymc

* update tests, update aesara import
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