Media transformation class and different transformations based on subsets of channels #968

wd60622 · 2024-08-23T02:51:41Z

Description

Allow for media transformation on subset of channels

Related Issue

Closes Different media transformations across different channels #712
Related to #

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

codecov · 2024-08-23T03:15:58Z

Codecov Report

Attention: Patch coverage is 89.04110% with 8 lines in your changes missing coverage. Please review.

Project coverage is 95.48%. Comparing base (cafa3f8) to head (46192b6).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
pymc_marketing/mmm/media_transformation.py	88.23%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #968      +/-   ##
==========================================
- Coverage   95.62%   95.48%   -0.14%     
==========================================
  Files          36       37       +1     
  Lines        3517     3590      +73     
==========================================
+ Hits         3363     3428      +65     
- Misses        154      162       +8

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

juanitorduz · 2024-08-23T17:36:11Z

pymc_marketing/mmm/__init__.py

+    MediaConfig,
+    MediaConfigs,


Shall we have more different names? Just as "s" at the end is a typo-rick hehe. For example MediaConfig and MediaConfigList?

Good idea 😆

juanitorduz · 2024-08-23T17:37:40Z

pymc_marketing/mmm/media_transformation.py

+    ]
+
+
+Apply the media transformation to media data in PyMC model:


Can this configuration specification be used through the media_config parameter with the MMM model class? If so, could you provide an example here as well 🙏 ?

There isn't a media_config parameter at the moment. But we could make one 👀

juanitorduz

Wow! Super Cool! Just left some questions.

wd60622 · 2024-08-24T11:08:38Z

pymc_marketing/mmm/media_transformation.py

+MediaConfigs = list[MediaConfig]
+
+
+def apply_media_transformation(


If these are wrapped in a class, then it should be pretty easy to make a consistent API with the apply method.

WDYT?

wd60622 · 2024-08-24T11:14:28Z

pymc_marketing/mmm/media_transformation.py

+            else (self.saturation, self.adstock)
+        )
+
+    def __call__(self, x, dim):


I use call here instead of an apply method used in individual transformations. WYDT?
As mentioned below, we can have

media_data = ... online_media_transformation = MediaTransformation(adstock=..., saturation=..., ...) offline_media_transformation = MediaTransformation(adstock=..., saturation=..., ...) media_configs = [MediaConfig(media_transformation=online_media_transformation), MediaConfig(media_transformation=offline_media_transformation)] # Wrapper around current MediaConfigs comb_media_transformation = CombinedMediaTransformation(media_configs=media_configs) # Make all three work the same offline_media_transformation(media_data, dims="media") online_media_transformation(media_data, dims="media") comb_media_transformation(media_data, dims="media")

This would have have many conveniences

…ifferent-channels

wd60622 · 2024-08-29T15:52:05Z

I've addressed all the feedback @juanitorduz except for the integration with MMM. That would require a handful of adjustments and discussion.

For instance,
is this a new parameter for initialization?

media_transformation = MediaTransformation(...)
mmm = MMM(media_transformation=media_transformation, ...)

media_transformation = MediaConfigList(...)
mmm = MMM(media_transformation=media_transformation, ...)

That has some implications:

what do we do with current parameters adstock, saturation, adstock_first? Deprecate?
How do we handle all of the model_config parameters? Should we store all of ModelTransformation / ModelConfigList there even though that is duplicated since there are stored in the individual transformations themselves
Various plotting functions need to be rewritten in order to support multiple adstocks / saturations etc
Budget optimizer will only conditionally work if single MediaTransformation

Definitely doable though

Do you want to do now or wait on this? Let me know how you imagine this being added in

juanitorduz

Yay! We should work on iterstions so let's merge this one and leave the MMM integration for another PR (can you please create an issue so that we keep track?)

…ifferent-channels

wd60622 · 2024-08-29T20:30:54Z

Yay! We should work on iterstions so let's merge this one and leave the MMM integration for another PR (can you please create an issue so that we keep track?)

Yes, will make some issues now.

Would you like me to increase the test coverage?

juanitorduz · 2024-08-29T20:33:20Z

@wd60622 from my side it looks ok! If you wanna test edge cases go for it.

wd60622 · 2024-08-29T20:34:12Z

@wd60622 from my side it looks ok! If you wanna test edge cases go for it.

I will create an issue for it

…sets of channels (pymc-labs#968)

…sets of channels (#968)

add media transformation logic

fc6958f

wd60622 added enhancement New feature or request MMM labels Aug 23, 2024

wd60622 added this to the 0.9.0 milestone Aug 23, 2024

wd60622 linked an issue Aug 23, 2024 that may be closed by this pull request

Different media transformations across different channels #712

Closed

wd60622 added 5 commits August 23, 2024 02:54

Update UML Diagrams

a7fb062

change of docstring imports

f7df3eb

add to the __init__

f91946b

Update UML Diagrams

6777e24

add to docstrings

9d8123a

write basic tests

e1336eb

wd60622 requested a review from juanitorduz August 23, 2024 09:27

juanitorduz reviewed Aug 23, 2024

View reviewed changes

wd60622 commented Aug 24, 2024

View reviewed changes

wd60622 and others added 7 commits August 29, 2024 09:23

resolve conflicts

7a6e9f5

Merge branch 'main' into 712-different-media-transformations-across-d…

af753b9

…ifferent-channels

add convenience method

4c995a3

change the API

7b57fd0

changes to test

744928a

serialization methods

847b1c5

helper equality method

54df0dd

juanitorduz approved these changes Aug 29, 2024

View reviewed changes

Merge branch 'main' into 712-different-media-transformations-across-d…

46192b6

…ifferent-channels

This was referenced Aug 29, 2024

Scope using MediaTransformation and MediaConfigList in MMM class #984

Open

MediaTransformation and MediaConfigList in example notebook #985

Open

wd60622 merged commit f2318ae into main Aug 29, 2024
11 of 12 checks passed

radiokosmos pushed a commit to radiokosmos/pymc-marketing that referenced this pull request Sep 3, 2024

Media transformation class and different transformations based on sub…

e7cc642

…sets of channels (pymc-labs#968)

twiecki pushed a commit that referenced this pull request Sep 10, 2024

Media transformation class and different transformations based on sub…

b7c66cd

…sets of channels (#968)

twiecki pushed a commit that referenced this pull request Sep 10, 2024

Media transformation class and different transformations based on sub…

3d334bc

…sets of channels (#968)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Media transformation class and different transformations based on subsets of channels #968

Media transformation class and different transformations based on subsets of channels #968

wd60622 commented Aug 23, 2024 •

edited

Loading

codecov bot commented Aug 23, 2024 •

edited

Loading

juanitorduz Aug 23, 2024

wd60622 Aug 24, 2024

juanitorduz Aug 23, 2024

wd60622 Aug 24, 2024

juanitorduz left a comment

wd60622 Aug 24, 2024

juanitorduz Aug 25, 2024

wd60622 Aug 24, 2024 •

edited

Loading

juanitorduz Aug 25, 2024

wd60622 commented Aug 29, 2024

juanitorduz left a comment

wd60622 commented Aug 29, 2024

juanitorduz commented Aug 29, 2024

wd60622 commented Aug 29, 2024

		]


		Apply the media transformation to media data in PyMC model:

		MediaConfigs = list[MediaConfig]


		def apply_media_transformation(

Media transformation class and different transformations based on subsets of channels #968

Media transformation class and different transformations based on subsets of channels #968

Conversation

wd60622 commented Aug 23, 2024 • edited Loading

Description

Related Issue

Checklist

Modules affected

Type of change

codecov bot commented Aug 23, 2024 • edited Loading

Codecov Report

juanitorduz Aug 23, 2024

Choose a reason for hiding this comment

wd60622 Aug 24, 2024

Choose a reason for hiding this comment

juanitorduz Aug 23, 2024

Choose a reason for hiding this comment

wd60622 Aug 24, 2024

Choose a reason for hiding this comment

juanitorduz left a comment

Choose a reason for hiding this comment

wd60622 Aug 24, 2024

Choose a reason for hiding this comment

juanitorduz Aug 25, 2024

Choose a reason for hiding this comment

wd60622 Aug 24, 2024 • edited Loading

Choose a reason for hiding this comment

juanitorduz Aug 25, 2024

Choose a reason for hiding this comment

wd60622 commented Aug 29, 2024

juanitorduz left a comment

Choose a reason for hiding this comment

wd60622 commented Aug 29, 2024

juanitorduz commented Aug 29, 2024

wd60622 commented Aug 29, 2024

wd60622 commented Aug 23, 2024 •

edited

Loading

codecov bot commented Aug 23, 2024 •

edited

Loading

wd60622 Aug 24, 2024 •

edited

Loading