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.

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

Add BetaGeoBetaBinomModel #922

Closed
wants to merge 190 commits into from
Closed

Conversation

ColtAllen
Copy link
Collaborator

@ColtAllen ColtAllen commented Aug 11, 2024

Description

The BG/BB model is for non-contractual purchase opportunities across discrete time periods; a good example would be sporting events.

This PR is a draft because I still need to add docstrings and debug the distribution_new_customer methods, but I could also use some assistance with a broadcasting issue I'm encountering with the logp method (error trace provided below). I also can't persist a model at all until either model configs support pm.Deterministic, or a different default model setup is used (see issue below).

Related Issue

Checklist

Modules affected

  • MMM
  • CLV

Type of change

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

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

wd60622 and others added 30 commits April 2, 2024 15:07
* add scaling into lift test method

* test scaling methods

* test change of MMM likelihood

* link up beta_channel parameters

* change name to sigma

* reorganize and edit

* closes #406

* address the feedback in docstrings

* add more to docstring

* format the docstring

* be verbose for future devs

* be explicit for the column max values

* incorporate the feedback

* hide cells and add to intro

* add conclusion

* switch to header 2

* run notebook through

* move the types together

* separate model estimated from empirical

* fix typos
* drop python 3.9

* try python 3.12

* undo try python 3.12
* improve nb

* rm warnings and add link to lifetimes quickstart

* address comments

* feedback part 3

* remove warnings manually
* add more info to the notebook

* hide plots code

* fix plot y labels

* fix plot outputs and remove model build

* improve final note probability plots

* address comments

* use quickstart dataset

* feedback part 3

* remowe warnings manually

* feedback part 4
* improve mmm docs init

* add more code examples to docstrings

* minor improvemeents

* typo

* better phrasing

* add thomas suggestion
* move fixtures to conftest

* docstrings and moved set_model_fit to conftest

* fixed pandas quickstart warnings

* revert to MockModel and add ParetoNBD support

* quickstart edit for issue 609

* notebook edit
remove ruff E501 ignore
updates:
- [github.com/astral-sh/ruff-pre-commit: v0.3.5 → v0.3.7](astral-sh/ruff-pre-commit@v0.3.5...v0.3.7)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Juan Orduz <[email protected]>
* fix potential bugs

* minor improvements

* remove rule for nb

* fix test

* improve tests syntax

* use stacklevel 2 for warnings

* use stacklevel 1 for warnings as they are used in public methods

* ignore B904

* undo change

* ricardos feedback

* use fit_posterior

* Update pymc_marketing/clv/models/gamma_gamma.py

Co-authored-by: Ricardo Vieira <[email protected]>

* last fix XD

---------

Co-authored-by: Ricardo Vieira <[email protected]>
* notebook opening and imports

* model definition markdown

* Data Load Notebook section

* WIP model fitting section

* added notebook to docs directory

* notebook edits and graph code

* ppc section and nb cleanup

* demz sampling and WIP plotting

* WIP predictive plots

* WIP heatmap plots

* predictive plots

* WIP covariates and nbqa-ruff edits

* covariate section

* plot additions

* fig sizes

* remove model file
…book (#651)

* add spaces, increase indentation, and fix number order

* explicit with 6
* Creating plot waterfall

Co-Authored-By: Carlos Trujillo <[email protected]>

* requested changes

* pre-commit

---------

Co-authored-by: Carlos Trujillo <[email protected]>
Databricks should have a lower-case b.
updates:
- [github.com/astral-sh/ruff-pre-commit: v0.4.1 → v0.4.2](astral-sh/ruff-pre-commit@v0.4.1...v0.4.2)
- [github.com/pre-commit/mirrors-mypy: v1.9.0 → v1.10.0](pre-commit/mirrors-mypy@v1.9.0...v1.10.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* support for negative values and dates (if used)

* fix terrible spelling

* test dates in coords

* cover numpy objects

* consolidate the tests
wd60622 and others added 13 commits August 24, 2024 14:00
* deprecations and moving files

* Update UML Diagrams

* change the imports in notebooks

* push up the code / test changes. need to run

* remove _get_\w*_function tests

* rerun the tvp notebook

* remove stale test

* move away from string initialization

* change the tvp media example
* allow register and use custom transform

* add to the example block
updates:
- [github.com/astral-sh/ruff-pre-commit: v0.6.1 → v0.6.2](astral-sh/ruff-pre-commit@v0.6.1...v0.6.2)
- [github.com/pre-commit/mirrors-mypy: v1.11.1 → v1.11.2](pre-commit/mirrors-mypy@v1.11.1...v1.11.2)

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

* sync index

* empty

* improvements
* nb init

* add param stability

* crps init

* add crps to package

* make mypy happy

* clean

* relax req

* update path

* add crps test

* first iteration

* improvements

* improve tests

* improve tests

* improve references

* feedback 1

* add examples
…ocs (#992)

* Changed pt=pt to pt_lib = None to avoid showing full module path in docs

* Changed pt to pt_lib inside calculate_lift_measurements_from_curve
* push up some linear-trend work I had

* image for the documentation

* start the slopes at t=0

* additional checks at init

* import at mmm module

* update the image

* Move to reference section

* Move coords to single line

* Add hierarchical trend example

* add image for the example

* migrate to pydantic

* update images after seed

* Correct the latex

* fix check from tests
updates:
- [github.com/astral-sh/ruff-pre-commit: v0.6.2 → v0.6.3](astral-sh/ruff-pre-commit@v0.6.2...v0.6.3)

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

@ricardoV94 ricardoV94 left a comment

Choose a reason for hiding this comment

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

Checking the error now, but left some comments in the code after skimming through it. Also the dev notebook is failing, I guess it is known, but flagging just in case

@ricardoV94
Copy link
Contributor

@ColtAllen I pushed a commit that allows working with batch chain/draws samples in the _logp method. The tests test_expected_purchases and test_expected_probability_alive are now passing locally for me.

I had to lower the rtol because the test is using less data sample_data, but otherwise the values seem to agree nicely with the expectation.

Unrelated, the test test_distribution_new_customer seems to be failing, and it doesn't seem just a tolerance problem. Perhaps as the comment in the testcode says, you are missing some letters / parametrization?

@ColtAllen
Copy link
Collaborator Author

Thanks! I just need to finish the docstrings and we can get this merged. The dev notebook is only for testing right now, but I'll expand it into something worthy of adding to the docs in a separate PR.

Unrelated, the test test_distribution_new_customer seems to be failing, and it doesn't seem just a tolerance problem. Perhaps as the comment in the testcode says, you are missing some letters / parametrization?

Yes, after double-checking, delta and gamma were swapped in the test. This is now passing.

@juanitorduz
Copy link
Collaborator

thanks @ricardoV94 !

@juanitorduz juanitorduz added this to the 0.9.0 milestone Sep 5, 2024
@ColtAllen ColtAllen closed this by deleting the head repository Sep 10, 2024
@ColtAllen
Copy link
Collaborator Author

Had to close this one due to the force push. Will be opening a new PR once ready for review.

@juanitorduz
Copy link
Collaborator

Had to close this one due to the force push. Will be opening a new PR once ready for review.

Fantastic! Thanks @ColtAllen

@ColtAllen ColtAllen mentioned this pull request Sep 13, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLV enhancement New feature or request help wanted Extra attention is needed priority: medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.