Add Covariates to `ParetoNBDModel` #463

ColtAllen · 2023-12-13T01:19:35Z

Description

This PR provides the option of using time-invariant covariates with ParetoNBDModel. This is a draft PR for now because I still need to parametrize some tests and add a few other things, but more importantly get opinions on two points:

Covariates require scaling for model convergence, preferably with MinMaxScaler so the coefficients are interpretable. This can easily be added as a utility function, and the covariate Min/Max values saved as model.idata attributes for out-of-sample predictions. Should a scaler be added in this PR, or a future one? Should it be added to ParetoNBDModel , or the base CLVModel?

Covariates for existing customers will also require modifications to the "new customer" methods. There are three options for this:

Just raise a ValueError indicating these methods aren't supported with covariates (easy, but not my preference.)
Average all the customer-specific alpha and beta parameters together for new customer estimates, which is a subset of the next option.
Add arguments to these methods for user-specified covariate values. This will be involved and maybe even clunky to use in practice, but if a user wants to do sensitivity analysis on how covariates can impact new customer estimates, this is the way to go.

Regardless of the approach, should it added to this PR, or another one? This one is already rather lengthy.

Related Issue

Closes #
Related to # Additional enhancements for ParetoNBDModel #183, Add Time-Invariant Covariates to CLV Models #134

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--463.org.readthedocs.build/en/463/

…eting into pareto_covar

review-notebook-app · 2023-12-13T01:19:40Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

codecov · 2023-12-13T01:39:02Z

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (a224cfe) 91.36% compared to head (5546f1f) 90.07%.

❗ Current head 5546f1f differs from pull request most recent head a63b0bd. Consider uploading reports for the commit a63b0bd to get more accurate results

Files	Patch %	Lines
pymc_marketing/clv/models/pareto_nbd.py	80.00%	20 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #463      +/-   ##
==========================================
- Coverage   91.36%   90.07%   -1.30%     
==========================================
  Files          21       21              
  Lines        2073     2025      -48     
==========================================
- Hits         1894     1824      -70     
- Misses        179      201      +22

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

ColtAllen · 2023-12-13T08:53:38Z

Tests are all passing, albeit incomplete. The CI seems to have timed out on an MMM test.

twiecki · 2024-01-08T07:17:16Z

Ready for review?

ColtAllen · 2024-01-08T14:27:45Z

Ready for review?

I'm gonna save the covariate scaling and new customer modifications for future PRs. Not all tests are parametrized yet for covariates, and predictive methods will fail for out-of-sample predictions.

ColtAllen · 2024-01-14T02:56:54Z

@ricardoV94 I could use your help with an xarray dimensional error in the expected_purchases and expected_probability_alive methods:

 ValueError: cannot reindex or align along dimension 'customer_id' 
because of conflicting dimension sizes: {2356, 2357} 
(note: an index is found along that dimension with size=2356)

Otherwise, this is ready for review.

pymc_marketing/clv/models/pareto_nbd.py

juanitorduz · 2024-01-17T20:43:51Z

pymc_marketing/clv/models/pareto_nbd.py

+        """
+        Utility function to process covariates into model parameters.
+        """
+        # TODO: broadcasting needs to be resolved for out-of-sample data


Do you want to add a Warning to handle this TODO in case is not part of the PR?

pymc_marketing/clv/models/pareto_nbd.py

juanitorduz · 2024-01-17T20:46:44Z

@ColtAllen This is very cool! I need (and will!) to catch up with the development! I left some basic comments on the PR. I hope I can contribute more once I read the model details 🙏

pymc_marketing/clv/models/pareto_nbd.py

ricardoV94 · 2024-01-18T13:23:30Z

tests/clv/models/test_pareto_nbd.py

@@ -265,6 +356,8 @@ def test_expected_probability_alive(self):
        est_prob_alive_t = self.model.expected_probability_alive(future_t=4.5)
        assert est_prob_alive.mean() > est_prob_alive_t.mean()

+        self.covar_model.expected_probability_alive()


Test something about this. Doesn't the same question of covariates show up here?

xarray issue needs to be resolved first (see my latest comment).

ricardoV94 · 2024-01-18T13:24:25Z

tests/clv/models/test_pareto_nbd.py

@@ -290,6 +383,8 @@ def test_expected_purchase_probability(self, test_n, test_t):
            rtol=0.001,
        )

+        self.model.expected_purchase_probability(test_n, test_t, self.data)


Did you mean self.covar_model? Also need to test something about the output

xarray issue needs to be resolved first (see my latest comment).

tests/clv/models/test_pareto_nbd.py

ricardoV94 · 2024-01-18T13:28:22Z

Just raise a ValueError indicating these methods aren't supported with covariates (easy, but not my preference.)

Average all the customer-specific alpha and beta parameters together for new customer estimates, which is a subset of the next option.

Add arguments to these methods for user-specified covariate values. This will be involved and maybe even clunky to use in practice, but if a user wants to do sensitivity analysis on how covariates can impact new customer estimates, this is the way to go.

I don't like option 2. I would say 1 if you want to get this merged quickly, and 3 otherwise (or later)

ricardoV94 · 2024-01-18T13:28:57Z

@ricardoV94 I could use your help with an xarray dimensional error in the expected_purchases and expected_probability_alive methods:
 ValueError: cannot reindex or align along dimension 'customer_id' 
because of conflicting dimension sizes: {2356, 2357} 
(note: an index is found along that dimension with size=2356)
Otherwise, this is ready for review.

What code can I look at that runs this error? The dev notebook/ a specific test?

ColtAllen · 2024-01-30T03:35:29Z

 ValueError: cannot reindex or align along dimension 'customer_id' 
because of conflicting dimension sizes: {2356, 2357} 
(note: an index is found along that dimension with size=2356)

What code can I look at that runs this error? The dev notebook/ a specific test?

Tests are failing at these lines:
https://github.com/ColtAllen/pymc-marketing/blob/pareto_covar/pymc_marketing/clv/models/pareto_nbd.py#L687

https://github.com/ColtAllen/pymc-marketing/blob/pareto_covar/pymc_marketing/clv/models/pareto_nbd.py#L639

https://github.com/ColtAllen/pymc-marketing/blob/pareto_covar/pymc_marketing/clv/models/pareto_nbd.py#L554

ColtAllen · 2024-01-30T03:48:48Z

I don't know how important this is to resolve, but PyCharm is highlighting this inconsistency:

Signature of method 'ParetoNBDModel.build_model()' does not match signature of the base method in class 'ModelBuilder' 
 Inspection info:
Reports inconsistencies in overriding method signatures.
Example:
class Book:
    def add_title(self):
        pass


class Novel(Book):
    def add_title(self, text):
        pass
Parameters of the add_title method in the Novel class do not match the method signature specified in the Book class. As a fix, the IDE offers to apply the Change Signature refactoring.

ColtAllen and others added 20 commits November 7, 2023 13:36

test dataset, docstrings, WIP covariate processing

3b76071

checkpoint for model covariate coordinates

66b6a65

column validation function and notebook eda

0d8858d

mypy test

010e881

removed explicit mypy typing from predict methods

e8bc438

checkpoint test params and default_model_config

a851c69

WIP test revisions

8e2a486

_process_covariates added

fba53b3

Cleaned up __init__ and added future TODOs

53c463b

WIP covariate model coordinates and scaler

6f184a7

_process_covariates returns xarrays and notebook dev

ada2b5b

Notebook experiments

cd28051

Merge branch 'pymc-labs:main' into pareto_covar

b8a750a

Merge branch 'pymc-labs:main' into pareto_covar

90155d4

Merge branch 'pareto_covar' of https://github.com/ColtAllen/pymc-mark…

1495221

…eting into pareto_covar

WIP new_customer_distributions

3ee3633

WIP new_customer_distributions

ff22a09

custom prior ndims and save_load fixed

e91a9ce

covariate array broadcasting and notebook dev work

fdce9c7

TODO comments

a24510e

ColtAllen added enhancement New feature or request CLV request discussion major API breaking changes labels Dec 13, 2023

ColtAllen requested a review from ricardoV94 December 13, 2023 01:19

ColtAllen self-assigned this Dec 13, 2023

Merge branch 'pymc-labs:main' into pareto_covar

ac16f2f

twiecki marked this pull request as ready for review January 8, 2024 07:17

Merge branch 'pymc-labs:main' into pareto_covar

da4207e

ColtAllen mentioned this pull request Jan 11, 2024

Rename clv_summary to rfm_summary and extend functionality #479

Merged

12 tasks

ColtAllen added 2 commits January 13, 2024 15:05

TODOs and covar test setup

5546f1f

WIP predictive methods

0c5e8b4

juanitorduz reviewed Jan 17, 2024

View reviewed changes

pymc_marketing/clv/models/pareto_nbd.py Outdated Show resolved Hide resolved

juanitorduz reviewed Jan 17, 2024

View reviewed changes

pymc_marketing/clv/models/pareto_nbd.py Outdated Show resolved Hide resolved

juanitorduz reviewed Jan 17, 2024

View reviewed changes

pymc_marketing/clv/models/pareto_nbd.py Show resolved Hide resolved

ricardoV94 removed the major API breaking changes label Jan 18, 2024

ricardoV94 reviewed Jan 18, 2024

View reviewed changes

variable renaming and docstrings

a9561b4

reuse alpha and beta priors for covariates

35b7106

Merge branch 'pymc-labs:main' into pareto_covar

fd3d148

ColtAllen mentioned this pull request Feb 4, 2024

CLV Naming Conventions and Test Data #506

Closed

11 tasks

Merge branch 'pymc-labs:main' into pareto_covar

a63b0bd

ColtAllen mentioned this pull request Feb 9, 2024

Publish ParetoNBDModel Notebook #524

Closed

ricardoV94 mentioned this pull request Feb 22, 2024

Implement ParetoNBD with covariates #545

Merged

13 tasks

ricardoV94 closed this in #545 Mar 15, 2024

ColtAllen deleted the pareto_covar branch June 1, 2024 05:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Covariates to `ParetoNBDModel` #463

Add Covariates to `ParetoNBDModel` #463

ColtAllen commented Dec 13, 2023 •

edited by github-actions bot

Loading

review-notebook-app bot commented Dec 13, 2023

codecov bot commented Dec 13, 2023 •

edited

Loading

ColtAllen commented Dec 13, 2023

twiecki commented Jan 8, 2024

ColtAllen commented Jan 8, 2024 •

edited

Loading

ColtAllen commented Jan 14, 2024

juanitorduz Jan 17, 2024

juanitorduz commented Jan 17, 2024

ricardoV94 Jan 18, 2024

ColtAllen Jan 30, 2024

ricardoV94 Jan 18, 2024

ColtAllen Jan 30, 2024

ricardoV94 commented Jan 18, 2024

ricardoV94 commented Jan 18, 2024

ColtAllen commented Jan 30, 2024

ColtAllen commented Jan 30, 2024

Add Covariates to ParetoNBDModel #463

Add Covariates to ParetoNBDModel #463

Conversation

ColtAllen commented Dec 13, 2023 • edited by github-actions bot Loading

Description

Related Issue

Checklist

Modules affected

Type of change

review-notebook-app bot commented Dec 13, 2023

codecov bot commented Dec 13, 2023 • edited Loading

Codecov Report

ColtAllen commented Dec 13, 2023

twiecki commented Jan 8, 2024

ColtAllen commented Jan 8, 2024 • edited Loading

ColtAllen commented Jan 14, 2024

juanitorduz Jan 17, 2024

Choose a reason for hiding this comment

juanitorduz commented Jan 17, 2024

ricardoV94 Jan 18, 2024

Choose a reason for hiding this comment

ColtAllen Jan 30, 2024

Choose a reason for hiding this comment

ricardoV94 Jan 18, 2024

Choose a reason for hiding this comment

ColtAllen Jan 30, 2024

Choose a reason for hiding this comment

ricardoV94 commented Jan 18, 2024

ricardoV94 commented Jan 18, 2024

ColtAllen commented Jan 30, 2024

ColtAllen commented Jan 30, 2024

Add Covariates to `ParetoNBDModel` #463

Add Covariates to `ParetoNBDModel` #463

ColtAllen commented Dec 13, 2023 •

edited by github-actions bot

Loading

codecov bot commented Dec 13, 2023 •

edited

Loading

ColtAllen commented Jan 8, 2024 •

edited

Loading