Harmonize prior and posterior predictive plots in MMM module

[juanitorduz]

Closes #1144

I simply extend the posterior predictive functionality to the prior predictive case without copy-paste but rather abstracting the plotting function by "group".

---

📚 Documentation preview 📚: https://pymc-marketing--1170.org.readthedocs.build/en/1170/

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[codecov]

Codecov Report

Attention: Patch coverage is 95.55556% with 2 lines in your changes missing coverage. Please review.

Project coverage is 95.59%. Comparing base (59a35cb) to head (0c6139a).
Report is 124 commits behind head on main.

Files with missing lines	Patch %	Lines
pymc_marketing/mmm/base.py	95.55%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1170      +/-   ##
==========================================
- Coverage   95.61%   95.59%   -0.03%     
==========================================
  Files          39       39              
  Lines        4063     4059       -4     
==========================================
- Hits         3885     3880       -5     
- Misses        178      179       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[juanitorduz]

I'll revert the error message change that is making a test fail.

[juanitorduz]

Can the end-to-end MM example also be updated to use this when conducting the prior predictive check?

Added in 53acf08 @cluhmann

[cluhmann]

Can I ask why there is an "observed" series plotted in the sample_prior_predictive() plot and why it's approximately zero (this is in the end to end MMM notebook)?

https://pymc-marketing--1170.org.readthedocs.build/en/1170/_images/2f0680c00d41aaca16ccd466cb2d48ba317aeef9f046c9da07ecaa81bfe225aa.png

[cluhmann]

(this is in the end to end MMM notebook)?

There is also an "observed" series in the MMM example notebook prior predictive plot.

[juanitorduz]

Can I ask why there is an "observed" series plotted in the sample_prior_predictive() plot and why it's approximately zero (this is in the end to end MMM notebook)?

https://pymc-marketing--1170.org.readthedocs.build/en/1170/_images/2f0680c00d41aaca16ccd466cb2d48ba317aeef9f046c9da07ecaa81bfe225aa.png

@cluhmann this is the observed series we (I?) usually compare with when I am doing the prior-predictive checks.

[cluhmann]

Ah. So the observed series looks like it's zero because it's variability is an order of magnitude smaller than the prior predictive plot?

[juanitorduz]

Exactly! I'll probably make the prior stricter.

[cluhmann]

I should have realized that. I need more coffee.

[juanitorduz]

@cluhmann can we merge this one so that I can tackle #1176 (git with. notebooks is a pain ;) )

[cluhmann]

I am rushing to get my pymc-marketing talk done (which is why I am running into these little documentation issues), so I can't give this a thorough review right now. But I probably could tomorrow (or someone like @wd60622 could). Sorry 😬

[juanitorduz]

all good! I will start solving in another branch the other issues (good luck in the talk ;) and thanks for all the feedback)

[wd60622]

I will give review today when I can

[wd60622]

Can we change the verbiage here as well. The use of "reasonable" is a bit strange

Also, would we want to comment on the fact that the prior predictive includes negative values... That might element to highlight

Reply via ReviewNB

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

wd60622 commented on 2024-11-06T20:35:15Z
----------------------------------------------------------------

Can we switch this out with Prior class. Also, why the change in prior for the other notebook and not here

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

wd60622 commented on 2024-11-06T20:35:16Z
----------------------------------------------------------------

Same idea here for conclusion. At least highlight that idea of negative density

[wd60622]

Just some commentary on the verbiage around the stuff mainly. Do you think the Likelihood distribution is something that we would like to change? Maybe first here in the notebook then in the defaults

WDYT?

[juanitorduz]

Just some commentary on the verbiage around the stuff mainly. Do you think the Likelihood distribution is something that we would like to change? Maybe first here in the notebook then in the defaults

WDYT?

Yes, let's look this into next iterations.

[juanitorduz]

@wd60622 I addressed your comments :)

[wd60622]

@wd60622 I addressed your comments :)

Let's ship it!

[wd60622]

Failing because of this: #1148