Deprecate and disable budget optimization method & Updating notebook #1832
Conversation
Marked the budget optimization method as deprecated and replaced its implementation with a NotImplementedError. Users are advised to migrate to the `Multidimensal.MMM` class.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1832 +/- ##
==========================================
- Coverage 92.20% 92.20% -0.01%
==========================================
Files 63 63
Lines 7312 7310 -2
==========================================
- Hits 6742 6740 -2
Misses 570 570 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| budget_bounds=budget_bounds, | ||
| callback=callback, | ||
| **minimize_kwargs, | ||
| raise NotImplementedError( |
There was a problem hiding this comment.
can we warn instead of raise?
There was a problem hiding this comment.
I think better raise, only because the current method even if it works will return incorrect results. No sense to keep it, but what do you think?
pymc_marketing/mmm/mmm.py
Outdated
| """Optimize the given budget based on the specified utility function over a specified time period (DEPRECATED). | ||
|
|
||
| This function optimizes the allocation of a given budget across different channels | ||
| DEPRECATED: This function optimizes the allocation of a given budget across different channels |
There was a problem hiding this comment.
There is a specific way to call out in numpydocs
https://numpydoc.readthedocs.io/en/latest/format.html#deprecation-warning
|
This complements #1849 deprecating method, meanwhile 1849 remove and update notebooks and dependencies. |
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
…ithub.com/pymc-labs/pymc-marketing into cetagostini/deprecating_optimizer_for_mmm
cetagostini
changed the title
|
@cetagostini i think its better to warn on the deprecation and remove in in 0.17.0 People are running these models in production and this is a key method. We will remove it for sure, but I think we need to do it in two steps. Thoughts? |
|
In the meantime… while warning. Can we somehow use the optimizer from the mmm multi clas I the old mmm class? |
Users can always pin to 0.15.1. Maybe raise the error and tell them to use 0.15.1? |
So, I could work on a PR to modify MMM base model to work as multidimensional, and be okay with the optimizer again. Nevertheless, probably I agree with Pablo that users who wants to continue with MMM base can always pin You dictate the course and I'll follow capitan! |
How much work would this be for you ? :) |
Maybe another day or two? (Meaning maybe like 5 days in OSS days 😅) Should I open the PR only adjusting scaling method on base MMM? |
|
I mean, I think we can wait a week (or two), there is really no rush. It would be great to modify MMM base model to work as multidimensional, if possible, in view that I know many people looking into the end-to-end notebook: https://www.pymc-marketing.io/en/latest/notebooks/mmm/mmm_case_study.html. Also, the MMM multidim still does not fully support the HSGP (see #1432). Hence, if we remove the optimizer, users won't be able to have a single model to fit and optimize. Hence, I suggest we modify MMM base model to work as multidimensional. No rush. Let me know if I can help. Does it make sense? |
|
Okay, I'll move this to draft, and will try to crack a PR for that during the next few days! |
Thank you @cetagostini ❤️ . I will buy you a beer in September (PyData Berlin) as a reward for this fantastic work with the optimizer! #hero |
Description
Marked the budget optimization method as deprecated and replaced its implementation with a NotImplementedError. Users are advised to migrate to the
Multidimensal.MMMclass.Related Issue
Checklist
pre-commit.ci autofixto auto-fix.📚 Documentation preview 📚: https://pymc-marketing--1832.org.readthedocs.build/en/1832/