Implement ParetoNBD with covariates

[ricardoV94]

Description

Related Issue

• Supersedes and closes Add Covariates to ParetoNBDModel #463
• 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

• New feature / enhancement
• Bug fix
• Documentation
• Maintenance
• Other (please specify):

---

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

[ColtAllen]

I think an updated UML diagram would be a helpful addition to CONTRIBUTING.MD to better understand all these new class inheritances:

Here are some instructions on how to create one:

#178

[codecov]

Codecov Report

Attention: Patch coverage is 19.73684% with 122 lines in your changes are missing coverage. Please review.

Project coverage is 35.61%. Comparing base (6f7a3d4) to head (db06d52).

Files	Patch %	Lines
pymc_marketing/clv/models/pareto_nbd.py	20.68%	115 Missing ⚠️
pymc_marketing/clv/models/beta_geo.py	0.00%	5 Missing ⚠️
pymc_marketing/clv/distributions.py	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #545       +/-   ##
===========================================
- Coverage   91.56%   35.61%   -55.96%     
===========================================
  Files          21       21               
  Lines        2052     2137       +85     
===========================================
- Hits         1879      761     -1118     
- Misses        173     1376     +1203     

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

[ColtAllen]

Code is certainly cleaner than the IF statement jungle of the PR I posted haha. Best way I can contribute is to create a child branch and work in parallel in the dev notebook for user-oriented feedback.

[ricardoV94]

Code is certainly cleaner than the IF statement jungle of the PR I posted haha. Best way I can contribute is to create a child branch and work in parallel in the dev notebook for user-oriented feedback.

It's always easier to refactor than write the first pass. The notebook would help a lot!

I'll be testing the covariate model today, and fix stuff I've certainly broken

[ricardoV94]

@ColtAllen finally pushed my last changes. I broke the pre-existing methods API very much, but we can:

1. Go back and make it like it looked before, plus optional argument for covariates in all methods
2. Use a new API but support old API with deprecation warnings
3. Something else. I don't like asking for a whole dataframe in cases where the user is only changing one variable, but for covariates it makes sense to request a dataframe anyway, so it makes sense that other variable also comes in the same dataframe

I'm in favor of 2.

Finally, the slow TestParetoNBDModelWithCovariates::test_inference is failing badly. I used synthetic data from fixed true parameters (so I could control the covariate coefficients), but it is not recovering those at all. Should I use different true parameter values?

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[ColtAllen]

@ColtAllen finally pushed my last changes. I broke the pre-existing methods API very much, but we can:

1. Go back and make it like it looked before, plus optional argument for covariates in all methods
2. Use a new API but support old API with deprecation warnings
3. Something else. I don't like asking for a whole dataframe in cases where the user is only changing one variable, but for covariates it makes sense to request a dataframe anyway, so it makes sense that other variable also comes in the same dataframe

I'm in favor of 2.

Yes, let's do option 2.

Finally, the slow TestParetoNBDModelWithCovariates::test_inference is failing badly. I used synthetic data from fixed true parameters (so I could control the covariate coefficients), but it is not recovering those at all. Should I use different true parameter values?

Yes - the true parameters you're using are for a model fit without covariates. When I fit a covariate model in the dev notebook of my previous PR, all parameter values changed.

I also noticed the covariates were synthesized and tacked onto an existing dataset. If we're to test via parameter recovery, we'll need to generate a full dataset from the parameters used for testing.

[ricardoV94]

I also noticed the covariates were synthesized and tacked onto an existing dataset. If we're to test via parameter recovery, we'll need to generate a full dataset from the parameters used for testing.

No, I'm generating new observations (recency and frequency) in that test as well

[ColtAllen]

No, I'm generating new observations (recency and frequency) in that test as well

Buy why is only a single prior predictive sample being taken?

[ricardoV94]

No, I'm generating new observations (recency and frequency) in that test as well

Buy why is only a single prior predictive sample being taken?

One draw is enough, it contains an observation for each synthetic customer

[ColtAllen]

No, I'm generating new observations (recency and frequency) in that test as well
One draw is enough, it contains an observation for each synthetic customer

I see. The new frequency column name is misspelled though, so it's testing on the original column rather than what was generated for the test.

[ColtAllen]

Is the user API still a work in progress? All predictive methods require arguments to be converted into dataframe columns beforehand, which seems rather cumbersome.