Open daniel-saunders-phil opened 3 months ago
Think this is a good idea. Probably want to use pymc / pytensor tooling though for the checks instead of numpy. The pymc.logprob.utils.CheckParameterValue
can be used.
Thoughts @juanitorduz ?
Yes! We should use pytensor.
Checklist:
Adstock Transformations:
Saturation Transformations
Any other parameters that would have a restricted domain?
Original message
The docstring says alpha must be between 0 and 1. But you can pass negative and larger alpha values and it will produce strange results, silently. Negative alpha subtracts impressions from each day. Adstock greater than 1 causes exponential accumulation of impressions over time. I can't see users ever finding this behaviour helpful. ``` import numpy as np from pymc_marketing.mmm.transformers import geometric_adstock geometric_adstock(np.array([100,200,50]),alpha = 1.1).eval() geometric_adstock(np.array([100,200,50]),alpha = -0.1).eval() ``` **Proposed solution**: Adding a check would make it easier to spot mistakes, especially in large hierarchical models with complex parameterizations. ``` """ if np.any(alpha >= 1) or np.any(alpha < 0): raise ValueError("alpha must be between greater than or equal to 0 and less than 1.") w = pt.power(pt.as_tensor(alpha)[..., None], pt.arange(l_max, dtype=x.dtype)) w = w / pt.sum(w, axis=-1, keepdims=True) if normalize else w return batched_convolution(x, w, axis=axis, mode=mode) ```