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 optimization notebook #358

Draft
wants to merge 128 commits into
base: main
Choose a base branch
from
Draft

Add optimization notebook #358

wants to merge 128 commits into from

Conversation

cluhmann
Copy link
Contributor

@cluhmann cluhmann commented Aug 17, 2023

Initial version of the optimization notebook. I have not yet added all the "bells and whistles" that are possible, because I want to get feedback on the core first. Then we can make it pretty.

One thing that I think can immediately be improved is to replace much of the manual contribution/ROAS calculation code with things like mmm.channel_contributions_forward_pass() etc. These helper functions didn't exist when the notebook was first created.

Also, I had planned on omitting all the MMM example notebook code, but realized I need to serialize the model, the inferenceData object, and the input data (and possibly more?). So I just kept the code but stripped it down to the essentials. Keeping all these cells also prevents "syncing" issues between this notebook and the MMM example notebook.

Todos:

  • Replace objective function code with pymc-marketing functionality as possible
  • Make objective function a compiled pytensor function to avoid eval()s
  • Add additional exploration of optimization results (e.g., risk attitudes, diversification penalty, etc.)

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

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.58%. Comparing base (ac645c9) to head (3b5c92a).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #358   +/-   ##
=======================================
  Coverage   94.58%   94.58%           
=======================================
  Files          33       33           
  Lines        3378     3378           
=======================================
  Hits         3195     3195           
  Misses        183      183           

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

wd60622 and others added 24 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
juanitorduz and others added 11 commits July 22, 2024 14:18
* make nb ruff astral-sh/ruff-vscode#546

* fix docs

* fix quickstart

* add types
* update uml diagrams

* add uml command to Makefile
* updated the ruff lint command, added ruff code formating command in Makefile, updated contributing.md

* fixing the issue with the new line at the end of makefile

* updated the PHONY list in makefile

---------

Co-authored-by: Murad Khalilov <[email protected]>
updates:
- [github.com/pre-commit/mirrors-mypy: v1.10.1 → v1.11.0](pre-commit/mirrors-mypy@v1.10.1...v1.11.0)

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

* Fix whitespace error

---------

Co-authored-by: Juan Orduz <[email protected]>
* add GH discussions link to README

* add link to MMM hub slack

* Update README.md

---------

Co-authored-by: Will Dean <[email protected]>
* Fix plotting by evaluating tensors.

* Add space after sphinx directive.

* Remove indentation from blank line.

* Add shared y axis for subplots.

---------

Co-authored-by: Patrick Robotham <[email protected]>
Co-authored-by: Will Dean <[email protected]>
* add original scale implementation

* add plot nb

* change location

* undo

* make mypy happy

* test plot

* add test

* update plot readme

* fix test

* improve variable description
* infernece changed to dataset

* inference changed dataset for plot_allocated_contribution_by_channel

---------

Co-authored-by: Will Dean <[email protected]>
* feat: adding root_saturation to transformers.py

* feat: adding RootSaturation class to saturation.py

* chore: adding missing RootSaturation to SATURATION_TRANSFORMATIONS

* feat: adding root_saturation to transformers.py

* feat: adding RootSaturation class to saturation.py

* chore: adding missing RootSaturation to SATURATION_TRANSFORMATIONS

* chore: linting edits

* chore: adding coefficient to function

* chore: linting corrections

* chore: removed empty References section of docstring

* chore: produce visual examples of root saturation

* chore: adding root to test_saturation.py

* chore: adding RootSaturation to init file

---------

Co-authored-by: ruari.walker <[email protected]>
Co-authored-by: Will Dean <[email protected]>
@wd60622
Copy link
Contributor

wd60622 commented Jul 25, 2024

This seems stale. Shall we close and turn into smaller issues?
Thoughts @cluhmann @juanitorduz

@juanitorduz
Copy link
Collaborator

This notebook has many great ideas that I would like to explore further. Can we leave it open while I try to get some time? Maybe we can add a special label?

@juanitorduz
Copy link
Collaborator

juanitorduz commented Jul 25, 2024

Or actually, as it is a dev notebook, can we merge it into the dev folder (as the clv dev ones)?

@cluhmann
Copy link
Contributor Author

Happy to help hammer this into something (more) useful.

@wd60622
Copy link
Contributor

wd60622 commented Jul 25, 2024

How about merge in since it is dev notebook and then create an issue or two?

@cluhmann
Copy link
Contributor Author

Fine with me.

wd60622 and others added 6 commits July 28, 2024 09:34
* separate the attr creation from attachment and perform check

* remove data for CLV

* fix model_builder tests

* fix clv tests

* more specific model builder checks

* rework with no args and kwargs

* rework common load method

* Update pymc_marketing/model_builder.py
* loads doesnt support boolean

* defaults for the media transformation

* test for the time_varyign
updates:
- [github.com/astral-sh/ruff-pre-commit: v0.5.4 → v0.5.5](astral-sh/ruff-pre-commit@v0.5.4...v0.5.5)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Update docs in transformers.py

docs: Fix the docstring

* Update docs in transformers.py

docs: fix other docstrings

* change the plot default

---------

Co-authored-by: Will Dean <[email protected]>
Co-authored-by: Will Dean <[email protected]>
* to_dict via lookup_name

* parse to and from dict for attrs

* improve the codecov

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

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

* change test with change in default behavior

* increase the MMM model version

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.