mock MMM posterior with prior #582

wd60622 · 2024-03-10T19:50:41Z

Description

Related Issue

Closes mock posterior with prior for MMM tests #518 Closes Migrate MMM plotting tests to test_base #86
Related to #

Checklist

Checked that the pre-commit linting/style checks pass
Included tests that prove the fix is effective or that the new feature works
Added necessary documentation (docstrings and/or example notebooks)
If you are a pro: each commit corresponds to a relevant logical change

Modules affected

MMM
CLV

Type of change

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

wd60622 · 2024-03-10T19:53:27Z

I have only two tests failing on my side because the values are now in different scale.

However, the tests run much faster 🎉 ~22mins -> 17mins -> 11mins
I am cutting down your coffee breaks 😉 @juanitorduz

wd60622 · 2024-03-14T19:03:35Z

I've mock the posterior with a prior

I've removed the two tests that were based on a "good" fit based on the input y:

the predictions are all positive
all within interval
I don't know if they were the best tests to begin with and I don't think these tests would make sense for the prior predictive

If there are any alternative suggestions, let me know. Would love to hear your thoughts @ricardoV94

Working on the actually fit tests that will be marked as slow. Are there any criteria that sticks out in your mind to test?

codecov · 2024-03-14T19:06:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.11%. Comparing base (7be19f6) to head (d8c49ce).

❗ Current head d8c49ce differs from pull request most recent head 90ff335

Please upload reports for the commit 90ff335 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #582      +/-   ##
==========================================
- Coverage   92.57%   91.11%   -1.47%     
==========================================
  Files          24       21       -3     
  Lines        2520     2172     -348     
==========================================
- Hits         2333     1979     -354     
- Misses        187      193       +6

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

ricardoV94 · 2024-03-15T08:26:12Z

tests/mmm/test_delayed_saturated_mmm.py

-    # Original scale constraint
-    assert np.all(posterior_predictive_mean >= 0)
-
-    # Domain kept close
-    lower, upper = np.quantile(a=posterior_predictive_mean, q=[0.025, 0.975], axis=0)
-    assert lower < toy_y.mean() < upper


Does it make sense for these tests not to work with the changes? They seem like rather useful checks

Commented this above. I will add these to the tests from the actual fit.
Using the normal likelihood doesn't guarantee the prior will meet these constraints. They were geared toward a fit model. Not a prior predictive

updates: - [github.com/astral-sh/ruff-pre-commit: v0.3.0 → v0.3.2](astral-sh/ruff-pre-commit@v0.3.0...v0.3.2) - [github.com/pre-commit/mirrors-mypy: v1.8.0 → v1.9.0](pre-commit/mirrors-mypy@v1.8.0...v1.9.0) - [github.com/nbQA-dev/nbQA: 1.8.3 → 1.8.4](nbQA-dev/nbQA@1.8.3...1.8.4)

Co-authored-by: Colt Allen <[email protected]>

wd60622 · 2024-03-21T17:14:28Z

Ran git pull and made a messy history. Sorry

I've added some slow tests to the MMM test suite that build of what was already existing but some additional checks like parameters being close to actuals

Let me know what you think of the quality of these tests! @juanitorduz

ricardoV94 · 2024-03-21T17:41:59Z

Ran git pull and made a messy history. Sorry

Probably you wanted git pull --rebase

wd60622 · 2024-03-21T17:57:56Z

Ran git pull and made a messy history. Sorry

Probably you wanted git pull --rebase

Fine if we just squash? Or shall I retry?

ricardoV94 · 2024-03-21T18:03:25Z

Don't retry :D

wd60622 · 2024-03-22T18:43:10Z

Only 1 test failing when trying to recover the original parameters. Trying to toy around with it as I think the scaling of the variables of X might be messing up the recovery.

Maybe we can save this for future issue with the speed gain from the non-slow tests

Wonder if you have any thoughts @juanitorduz . Any suggestions would be helpful

wd60622 · 2024-03-24T11:05:23Z

Test issue might be solved here @juanitorduz

tests/mmm/test_delayed_saturated_mmm.py

juanitorduz

Nice! I just left minor comments :) Can you please resolve the small conflicts with the main branch so that we can merge this one? :)

wd60622 · 2024-04-08T21:40:56Z

The test, test_distribution_customer_churn_time , is now failing. Would you know anything about this @ricardoV94 ? Maybe @ColtAllen

ricardoV94 · 2024-04-09T08:48:22Z

The test, test_distribution_customer_churn_time , is now failing. Would you know anything about this @ricardoV94 ? Maybe @ColtAllen

No idea why, seems like a meaningful failure. Better to open an issue about it

juanitorduz · 2024-04-09T08:50:16Z

The test, test_distribution_customer_churn_time , is now failing. Would you know anything about this @ricardoV94 ? Maybe @ColtAllen

No idea why, seems like a meaningful failure. Better to open an issue about it

It is open: #620

I checked and seems related to a pymc 5.12 -> 5.13 change (I tested locally and it started failing after the release)

ricardoV94 · 2024-04-09T08:51:30Z

The test, test_distribution_customer_churn_time , is now failing. Would you know anything about this @ricardoV94 ? Maybe @ColtAllen

No idea why, seems like a meaningful failure. Better to open an issue about it

It is open: #620

I checked and seems related to a pymc 5.12 -> 5.13 change (I tested locally and it started failing after the release)

You confirmed it passes on main with PyMC 5.12, and fails with 5.13? To rule out changes in the codebase here as well

wd60622

For context

wd60622 · 2024-04-11T17:18:24Z

tests/mmm/test_delayed_saturated_mmm.py

+def random_mask(df: pd.DataFrame, mask_value: float = 0.0) -> pd.DataFrame:
+    shape = df.shape
+
+    mask = rng.choice([0, 1], size=shape, p=[0.75, 0.25])
+    return df.mul(mask)
+
+
+@pytest.fixture(scope="module")
+def masked_toy_X(toy_X) -> pd.DataFrame:
+    return toy_X.set_index("date").pipe(random_mask).reset_index()


The rational here is that since all of the toy_X columns are pretty much white noise, the generated y is also pretty much a white noise and the model doesn't fit well

Here there are some clear stops in toy_X making the y notably depend on the covariates

tests/mmm/test_delayed_saturated_mmm.py

wd60622 · 2024-06-05T16:33:28Z

Closing as #632 introduces the mock_fit in the test suite. However, I've create #721 to follow up with some of the efforts here

mock the posterior with prior

d5b5176

wd60622 added MMM maintenance no releasenotes labels Mar 10, 2024

wd60622 marked this pull request as draft March 10, 2024 19:52

wd60622 added 2 commits March 11, 2024 00:10

rename and mock in test_plotting

c4f3884

remove tests that wont work with prior as posterior

f085838

wd60622 requested a review from ricardoV94 March 14, 2024 18:56

wd60622 marked this pull request as ready for review March 14, 2024 18:57

ricardoV94 reviewed Mar 15, 2024

View reviewed changes

ricardoV94 approved these changes Mar 18, 2024

View reviewed changes

ulfaslak and others added 16 commits March 21, 2024 18:08

Remove scale_preserving_logistic_saturation function (#585)

b30f9d3

Bump minimum Python version in environment.yml

6e7b701

Remove useless fixtures in test_gamma_gamma

29fde57

Make test_save_load lighter

ce80f57

Run slow tests in CI

6993997

Make GammaGamma.test_model_convergence more stable

277c7d1

Add test util for setting fake data in CLV models

49398fe

Implement ParetoNBD with covariates

b9e61d2

Co-authored-by: Colt Allen <[email protected]>

Update version.txt

13e6e9b

Make data optional in all ParetoNBD methods

4f33440

Assign coords in ParetoNBDModel

97b9807

Update version.txt

a6b6b94

add test setup

5c70875

store commit

7d0dd9c

store commit

267cef2

correct after failing

4823d39

ColtAllen mentioned this pull request Mar 24, 2024

Fix clv plotting bugs and edits to Quickstart #601

Merged

13 tasks

Merge branch 'main' into mock-mmm-fit

d8c49ce

wd60622 requested a review from juanitorduz April 1, 2024 19:45

juanitorduz reviewed Apr 8, 2024

View reviewed changes

tests/mmm/test_delayed_saturated_mmm.py Outdated Show resolved Hide resolved

juanitorduz reviewed Apr 8, 2024

View reviewed changes

tests/mmm/test_delayed_saturated_mmm.py Outdated Show resolved Hide resolved

juanitorduz reviewed Apr 8, 2024

View reviewed changes

wd60622 and others added 3 commits April 8, 2024 22:18

Merge branch 'main' into mock-mmm-fit

873436a

add variables that make up 500

8513e46

add an expected fail

34a79c8

wd60622 and others added 2 commits April 11, 2024 17:11

Merge branch 'pymc-labs:main' into mock-mmm-fit

0d36fd1

add additional difference between changes by masking

ed279c0

wd60622 commented Apr 11, 2024

View reviewed changes

wd60622 added 3 commits May 2, 2024 21:19

Merge branch 'main' into mock-mmm-fit

de5d5d2

Merge branch 'main' into mock-mmm-fit

1c86f78

Merge branch 'main' into mock-mmm-fit

90ff335

This was referenced Jun 5, 2024

User-defined media transformations and custom ordering #632

Merged

test against actual mmm fit results #721

Open

wd60622 closed this Jun 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mock MMM posterior with prior #582

mock MMM posterior with prior #582

wd60622 commented Mar 10, 2024 •

edited

Loading

wd60622 commented Mar 10, 2024 •

edited

Loading

wd60622 commented Mar 14, 2024 •

edited

Loading

codecov bot commented Mar 14, 2024 •

edited

Loading

ricardoV94 Mar 15, 2024

wd60622 Mar 15, 2024

wd60622 commented Mar 21, 2024

ricardoV94 commented Mar 21, 2024

wd60622 commented Mar 21, 2024

ricardoV94 commented Mar 21, 2024

wd60622 commented Mar 22, 2024 •

edited

Loading

wd60622 commented Mar 24, 2024

juanitorduz left a comment

wd60622 commented Apr 8, 2024

ricardoV94 commented Apr 9, 2024

juanitorduz commented Apr 9, 2024

ricardoV94 commented Apr 9, 2024

wd60622 left a comment

wd60622 Apr 11, 2024

wd60622 commented Jun 5, 2024

mock MMM posterior with prior #582

mock MMM posterior with prior #582

Conversation

wd60622 commented Mar 10, 2024 • edited Loading

Description

Related Issue

Checklist

Modules affected

Type of change

wd60622 commented Mar 10, 2024 • edited Loading

wd60622 commented Mar 14, 2024 • edited Loading

codecov bot commented Mar 14, 2024 • edited Loading

Codecov Report

ricardoV94 Mar 15, 2024

Choose a reason for hiding this comment

wd60622 Mar 15, 2024

Choose a reason for hiding this comment

wd60622 commented Mar 21, 2024

ricardoV94 commented Mar 21, 2024

wd60622 commented Mar 21, 2024

ricardoV94 commented Mar 21, 2024

wd60622 commented Mar 22, 2024 • edited Loading

wd60622 commented Mar 24, 2024

juanitorduz left a comment

Choose a reason for hiding this comment

wd60622 commented Apr 8, 2024

ricardoV94 commented Apr 9, 2024

juanitorduz commented Apr 9, 2024

ricardoV94 commented Apr 9, 2024

wd60622 left a comment

Choose a reason for hiding this comment

wd60622 Apr 11, 2024

Choose a reason for hiding this comment

wd60622 commented Jun 5, 2024

wd60622 commented Mar 10, 2024 •

edited

Loading

wd60622 commented Mar 10, 2024 •

edited

Loading

wd60622 commented Mar 14, 2024 •

edited

Loading

codecov bot commented Mar 14, 2024 •

edited

Loading

wd60622 commented Mar 22, 2024 •

edited

Loading