Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Behavior with shared lookup_name between AdstockTransformation and SaturationTransformation classes #1323

Open
wd60622 opened this issue Dec 28, 2024 · 0 comments
Labels
media transforms Related to adstock, saturation, and media transformations MMM request discussion

Comments

@wd60622
Copy link
Contributor

wd60622 commented Dec 28, 2024

Example:

from pymc_marketing.mmm import AdstockTransformation, SaturationTransformation
from pymc_marketing.deserialize import deserialize

same_lookup_name = "identity"

def identity(x):
    return x

class NewAdstock(AdstockTransformation): 
    lookup_name = same_lookup_name
    function = identity
    default_priors = {}

# This is allowed but unreachable with the deserialize function
class NewSaturation(SaturationTransformation):
    lookup_name = same_lookup_name
    function = identity
    default_priors = {}

data = {"lookup_name": same_lookup_name, "l_max": 10}
instance = deserialize(data)

What is the expected behavior here?

  1. Could be a call for a change in how they are serialized to make
    them more distinguishable.
NewAdstock().to_dict()
# {'lookup_name': 'identity',
#  'prefix': 'adstock',
#  'priors': {},
#  'l_max': 10,
#  'normalize': True,
#  'mode': 'After'}

NewSaturation().to_dict()
# {'lookup_name': 'identity',
#  'prefix': 'saturation',
#  'priors': {}}
  1. Or cross-reference the lookup_name with the others to make it unique across all classes.

I am in favor of the first option

@wd60622 wd60622 added MMM request discussion media transforms Related to adstock, saturation, and media transformations and removed Needs Triage labels Dec 28, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
media transforms Related to adstock, saturation, and media transformations MMM request discussion
Projects
None yet
Development

No branches or pull requests

1 participant