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.

Sign up for GitHub

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

Jump to bottom

Optimisation in customer_lifetime_value when discount_rate is zero #468

Merged
merged 2 commits into from
Jan 6, 2024
Merged

Optimisation in customer_lifetime_value when discount_rate is zero #468

merged 2 commits into from
Jan 6, 2024

Conversation

vincent-grosbois
Copy link
Contributor

@vincent-grosbois vincent-grosbois commented Dec 27, 2023 •
edited by ricardoV94
Loading

Optimisation in customer_lifetime_value when discount_rate == 0

closes #467

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

@vincent-grosbois vincent-grosbois mentioned this pull request Dec 27, 2023
Closed
@codecov Codecov
Copy link

codecov bot commented Jan 2, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6ecdb08) 90.82% compared to head (6f4483f) 90.83%.
Report is 3 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #468   +/-   ##
=======================================
  Coverage   90.82%   90.83%           
=======================================
  Files          21       21           
  Lines        1972     1974    +2     
=======================================
+ Hits         1791     1793    +2     
  Misses        181      181

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

vincent-grosbois
vincent-grosbois commented Jan 2, 2024
View reviewed changes
Copy link
Contributor Author

@vincent-grosbois vincent-grosbois left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whitespace fix

@juanitorduz juanitorduz requested a review from ricardoV94 January 4, 2024 19:26
@ricardoV94 ricardoV94 added enhancement New feature or request CLV labels Jan 5, 2024
ricardoV94
ricardoV94 reviewed Jan 5, 2024
View reviewed changes
Copy link
Contributor

@ricardoV94 ricardoV94 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @vincent-grosbois, thanks for the PR. If it's this simple, that's great.

Can you add a test? Perhaps you can compute the quantity with discount rate = 0, before the PR change, and then hard-code the results as expected results in a test. This will confirm we get the same numbers (or close enough if there are random draws involved) before and after the changes.

I don't think we had any test with discount rate = 0 before. If we had, then nothing should be needed.

@vincent-grosbois
Copy link
Contributor Author

I think there are some tests here :

pymc-marketing/tests/clv/test_utils.py

Line 150 in 2e826a9

clv_d0 = customer_lifetime_value(

which shows that nothing should have changed (because it's compared against some evaluations when discount_rate == 1, and this one didn't change)

@ricardoV94
Copy link
Contributor

Great we already tested the zero case so we're good

@ricardoV94 ricardoV94 merged commit dcc1ce7 into pymc-labs:main Jan 6, 2024
12 checks passed
@ricardoV94
Copy link
Contributor

Thanks for the contribution!

eirikbaekkelund pushed a commit to eirikbaekkelund/pymc-marketing that referenced this pull request Jan 7, 2024
@vincent-grosbois @eirikbaekkelund
Optimisation in customer_lifetime_value when discount_rate == 0 (pymc…
b3ebcfb 
…-labs#468)

* Optimisation in customer_lifetime_value when discount_rate == 0

cf pymc-labs#467

* Update utils.py
@ricardoV94 ricardoV94 added maintenance and removed enhancement New feature or request labels Jan 15, 2024
@ricardoV94 ricardoV94 changed the title Optimisation in customer_lifetime_value when discount_rate == 0 Optimisation in customer_lifetime_value when discount_rate iz zero Jan 15, 2024
@ricardoV94 ricardoV94 changed the title Optimisation in customer_lifetime_value when discount_rate iz zero Optimisation in customer_lifetime_value when discount_rate is zero Jan 15, 2024
juanitorduz added a commit that referenced this pull request Mar 8, 2024
@wd60622 @juanitorduz
@abdalazizrashid @ricardoV94 @vincent-grosbois @OriolAbril @ColtAllen @pre-commit-ci @ferrine
New spends forward pass (#456)
d5331be 
* current status as method

* format

* Update version.txt

* Implement different convolution modes (#454)

* Add PR template

* Update pull_request_template.md

* Fix issues in index example

* Update .pre-commit-config.yaml

* Update .pre-commit-config.yaml

* move from other PR

* put legend on side

* Optimisation in customer_lifetime_value when discount_rate == 0 (#468)

* Optimisation in customer_lifetime_value when discount_rate == 0

cf #467

* Update utils.py

* Update README.md

* add support for pre-commit-ci

* add isort

* modify autosummary templates

* Rename `clv_summary` to `rfm_summary` and extend functionality (#479)

* clv_summary adapted into rfm_summary

* added clv_summary with warning

* moved dataset from testing folder

* Update version.txt

* improve ruff

* [pre-commit.ci] pre-commit autoupdate

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.1.11 → v0.1.14](astral-sh/ruff-pre-commit@v0.1.11...v0.1.14)
- [github.com/pre-commit/pre-commit-hooks: v3.2.0 → v4.5.0](pre-commit/pre-commit-hooks@v3.2.0...v4.5.0)

* resolve conflict

* Add baselined saturation (#498)

* add baselined saturation with test and plots

* refactor docs

* add the reparam

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* verify parametrization is equivalent under change of baseline

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add a note for setting x0

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* make it clear how r_ref is calculated

* fix typo

* fix docstrings

* improve test by making sure transform is gives identical saturation and cac0

* add comment in the docstring

* add blank line in the code-block

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Swap Before and After convolution modes as per #489 (#501)

* Add support for string mode args

* Swap before and after and make mode explicit

* Use Union due Python 3.9

* Style

* resolve conflict

* add dim_name arg

* add seed to tests and test methods

* add slice as type hint

* use slice in docstring

* defaults to mean for each channel

* add non-negative check

* ax as last arg

* change weeks -> time

* parameterize quantiles

* separate out and add to docs

* rerun the baseline images

* mock the prior

* add new images from latest env

* migrate to toml instead of ci/cd

* test only is axes

* remove the images

---------

Co-authored-by: Juan Orduz <juan.orduz@wolt.com>
Co-authored-by: Abdalaziz Rashid <abdalaziz.rashid@outlook.com>
Co-authored-by: Ricardo Vieira <ricardo.vieira1994@gmail.com>
Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
Co-authored-by: vincent-grosbois <vincent.grosbois@gmail.com>
Co-authored-by: juanitorduz <juanitorduz@gmail.com>
Co-authored-by: Oriol (ProDesk) <oriol.abril.pla@gmail.com>
Co-authored-by: Colt Allen <10178857+ColtAllen@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Maxim Kochurov <max.kochurov@pymc-labs.com>
wd60622 added a commit that referenced this pull request Apr 8, 2024
@wd60622 @juanitorduz
@abdalazizrashid @ricardoV94 @vincent-grosbois @OriolAbril @ColtAllen @pre-commit-ci @ferrine
New spends forward pass (#456)
d3e4aa7 
* current status as method

* format

* Update version.txt

* Implement different convolution modes (#454)

* Add PR template

* Update pull_request_template.md

* Fix issues in index example

* Update .pre-commit-config.yaml

* Update .pre-commit-config.yaml

* move from other PR

* put legend on side

* Optimisation in customer_lifetime_value when discount_rate == 0 (#468)

* Optimisation in customer_lifetime_value when discount_rate == 0

cf #467

* Update utils.py

* Update README.md

* add support for pre-commit-ci

* add isort

* modify autosummary templates

* Rename `clv_summary` to `rfm_summary` and extend functionality (#479)

* clv_summary adapted into rfm_summary

* added clv_summary with warning

* moved dataset from testing folder

* Update version.txt

* improve ruff

* [pre-commit.ci] pre-commit autoupdate

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.1.11 → v0.1.14](astral-sh/ruff-pre-commit@v0.1.11...v0.1.14)
- [github.com/pre-commit/pre-commit-hooks: v3.2.0 → v4.5.0](pre-commit/pre-commit-hooks@v3.2.0...v4.5.0)

* resolve conflict

* Add baselined saturation (#498)

* add baselined saturation with test and plots

* refactor docs

* add the reparam

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* verify parametrization is equivalent under change of baseline

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add a note for setting x0

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* make it clear how r_ref is calculated

* fix typo

* fix docstrings

* improve test by making sure transform is gives identical saturation and cac0

* add comment in the docstring

* add blank line in the code-block

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Swap Before and After convolution modes as per #489 (#501)

* Add support for string mode args

* Swap before and after and make mode explicit

* Use Union due Python 3.9

* Style

* resolve conflict

* add dim_name arg

* add seed to tests and test methods

* add slice as type hint

* use slice in docstring

* defaults to mean for each channel

* add non-negative check

* ax as last arg

* change weeks -> time

* parameterize quantiles

* separate out and add to docs

* rerun the baseline images

* mock the prior

* add new images from latest env

* migrate to toml instead of ci/cd

* test only is axes

* remove the images

---------

Co-authored-by: Juan Orduz <juan.orduz@wolt.com>
Co-authored-by: Abdalaziz Rashid <abdalaziz.rashid@outlook.com>
Co-authored-by: Ricardo Vieira <ricardo.vieira1994@gmail.com>
Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
Co-authored-by: vincent-grosbois <vincent.grosbois@gmail.com>
Co-authored-by: juanitorduz <juanitorduz@gmail.com>
Co-authored-by: Oriol (ProDesk) <oriol.abril.pla@gmail.com>
Co-authored-by: Colt Allen <10178857+ColtAllen@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Maxim Kochurov <max.kochurov@pymc-labs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ricardoV94 ricardoV94 ricardoV94 approved these changes

Assignees
No one assigned
Labels
CLV maintenance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

possible optimization when discount_rate == 0
2 participants
@vincent-grosbois @ricardoV94