From ac8552d2cc06b30b7feb1aa61438177a4f306696 Mon Sep 17 00:00:00 2001 From: Pablo deRoque Date: Thu, 22 Aug 2024 16:39:39 +0200 Subject: [PATCH 1/8] Enforce check_parameters in geometric_adstock --- pymc_marketing/mmm/transformers.py | 6 ++++ tests/mmm/test_transformers.py | 45 ++++++++++++++++++++---------- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/pymc_marketing/mmm/transformers.py b/pymc_marketing/mmm/transformers.py index bb12b4b6a..55198cf32 100644 --- a/pymc_marketing/mmm/transformers.py +++ b/pymc_marketing/mmm/transformers.py @@ -20,6 +20,7 @@ import numpy.typing as npt import pymc as pm import pytensor.tensor as pt +from pymc.distributions.dist_math import check_parameters from pytensor.tensor.random.utils import params_broadcast_shapes @@ -235,6 +236,11 @@ def geometric_adstock( with carryover and shape effects." (2017). """ + alpha = check_parameters( + alpha, [pt.gt(alpha, 0), pt.lt(alpha, 1)], msg="0 <= alpha <= 1" + ).eval() + l_max = check_parameters(l_max, [pt.gt(l_max, 0)], msg="l_max >= 0").eval() + 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) diff --git a/tests/mmm/test_transformers.py b/tests/mmm/test_transformers.py index c1454c324..5b1e74e09 100644 --- a/tests/mmm/test_transformers.py +++ b/tests/mmm/test_transformers.py @@ -18,6 +18,7 @@ import pytensor.tensor as pt import pytest import scipy as sp +from pymc.logprob.utils import ParameterValueError from pytensor.tensor.variable import TensorVariable from pymc_marketing.mmm.transformers import ( @@ -131,22 +132,38 @@ def test_geometric_adstock_x_zero(self, mode): np.testing.assert_array_equal(x=x, y=y.eval()) @pytest.mark.parametrize( - "x, alpha, l_max", - [ - (np.ones(shape=(100)), 0.3, 10), - (np.ones(shape=(100)), 0.7, 100), - (np.zeros(shape=(100)), 0.2, 5), - (np.ones(shape=(100)), 0.5, 7), - (np.linspace(start=0.0, stop=1.0, num=50), 0.8, 3), - (np.linspace(start=0.0, stop=1.0, num=50), 0.8, 50), + "x, alpha, l_max, suceeds", + [ # succeeds + (np.ones(shape=(100)), 0.3, 10, True), + (np.ones(shape=(100)), 0.7, 100, True), + (np.zeros(shape=(100)), 0.2, 5, True), + (np.ones(shape=(100)), 0.5, 7, True), + (np.linspace(start=0.0, stop=1.0, num=50), 0.8, 3, True), + (np.linspace(start=0.0, stop=1.0, num=50), 0.8, 50, True), + # fails: bad alpha + (np.ones(shape=(100)), -1, 10, False), + (np.ones(shape=(100)), -0.5, 100, False), + (np.zeros(shape=(100)), 1.5, 5, False), + (np.ones(shape=(100)), 30000, 7, False), + (np.linspace(start=0.0, stop=1.0, num=50), -1, 3, False), + # fails: bad l_max + (np.ones(shape=(100)), 0.3, -10, False), + (np.ones(shape=(100)), 0.7, -100, False), + (np.zeros(shape=(100)), 0.2, -5, False), + (np.ones(shape=(100)), 0.5, -7, False), + (np.linspace(start=0.0, stop=1.0, num=50), 0.8, -3, False), ], ) - def test_geometric_adstock_good_alpha(self, x, alpha, l_max): - y = geometric_adstock(x=x, alpha=alpha, l_max=l_max) - y_np = y.eval() - assert y_np[0] == x[0] - assert y_np[1] == x[1] + alpha * x[0] - assert y_np[2] == x[2] + alpha * x[1] + (alpha**2) * x[0] + def test_geometric_adstock_check_parameters(self, x, alpha, l_max, suceeds): + if suceeds: + y = geometric_adstock(x=x, alpha=alpha, l_max=l_max) + y_np = y.eval() + assert y_np[0] == x[0] + assert y_np[1] == x[1] + alpha * x[0] + assert y_np[2] == x[2] + alpha * x[1] + (alpha**2) * x[0] + else: + with pytest.raises(ParameterValueError): + geometric_adstock(x=x, alpha=alpha, l_max=l_max) @pytest.mark.parametrize( argnames="mode", From 4bd7411e076f72649b6b5d55cdc048ea44bd2e87 Mon Sep 17 00:00:00 2001 From: Pablo deRoque Date: Thu, 22 Aug 2024 18:11:54 +0200 Subject: [PATCH 2/8] Remove check_parameters on l_max. Revert to original test --- pymc_marketing/mmm/transformers.py | 1 - tests/mmm/test_transformers.py | 45 ++++++++++-------------------- 2 files changed, 14 insertions(+), 32 deletions(-) diff --git a/pymc_marketing/mmm/transformers.py b/pymc_marketing/mmm/transformers.py index 55198cf32..49d35eed8 100644 --- a/pymc_marketing/mmm/transformers.py +++ b/pymc_marketing/mmm/transformers.py @@ -239,7 +239,6 @@ def geometric_adstock( alpha = check_parameters( alpha, [pt.gt(alpha, 0), pt.lt(alpha, 1)], msg="0 <= alpha <= 1" ).eval() - l_max = check_parameters(l_max, [pt.gt(l_max, 0)], msg="l_max >= 0").eval() 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 diff --git a/tests/mmm/test_transformers.py b/tests/mmm/test_transformers.py index 5b1e74e09..c1454c324 100644 --- a/tests/mmm/test_transformers.py +++ b/tests/mmm/test_transformers.py @@ -18,7 +18,6 @@ import pytensor.tensor as pt import pytest import scipy as sp -from pymc.logprob.utils import ParameterValueError from pytensor.tensor.variable import TensorVariable from pymc_marketing.mmm.transformers import ( @@ -132,38 +131,22 @@ def test_geometric_adstock_x_zero(self, mode): np.testing.assert_array_equal(x=x, y=y.eval()) @pytest.mark.parametrize( - "x, alpha, l_max, suceeds", - [ # succeeds - (np.ones(shape=(100)), 0.3, 10, True), - (np.ones(shape=(100)), 0.7, 100, True), - (np.zeros(shape=(100)), 0.2, 5, True), - (np.ones(shape=(100)), 0.5, 7, True), - (np.linspace(start=0.0, stop=1.0, num=50), 0.8, 3, True), - (np.linspace(start=0.0, stop=1.0, num=50), 0.8, 50, True), - # fails: bad alpha - (np.ones(shape=(100)), -1, 10, False), - (np.ones(shape=(100)), -0.5, 100, False), - (np.zeros(shape=(100)), 1.5, 5, False), - (np.ones(shape=(100)), 30000, 7, False), - (np.linspace(start=0.0, stop=1.0, num=50), -1, 3, False), - # fails: bad l_max - (np.ones(shape=(100)), 0.3, -10, False), - (np.ones(shape=(100)), 0.7, -100, False), - (np.zeros(shape=(100)), 0.2, -5, False), - (np.ones(shape=(100)), 0.5, -7, False), - (np.linspace(start=0.0, stop=1.0, num=50), 0.8, -3, False), + "x, alpha, l_max", + [ + (np.ones(shape=(100)), 0.3, 10), + (np.ones(shape=(100)), 0.7, 100), + (np.zeros(shape=(100)), 0.2, 5), + (np.ones(shape=(100)), 0.5, 7), + (np.linspace(start=0.0, stop=1.0, num=50), 0.8, 3), + (np.linspace(start=0.0, stop=1.0, num=50), 0.8, 50), ], ) - def test_geometric_adstock_check_parameters(self, x, alpha, l_max, suceeds): - if suceeds: - y = geometric_adstock(x=x, alpha=alpha, l_max=l_max) - y_np = y.eval() - assert y_np[0] == x[0] - assert y_np[1] == x[1] + alpha * x[0] - assert y_np[2] == x[2] + alpha * x[1] + (alpha**2) * x[0] - else: - with pytest.raises(ParameterValueError): - geometric_adstock(x=x, alpha=alpha, l_max=l_max) + def test_geometric_adstock_good_alpha(self, x, alpha, l_max): + y = geometric_adstock(x=x, alpha=alpha, l_max=l_max) + y_np = y.eval() + assert y_np[0] == x[0] + assert y_np[1] == x[1] + alpha * x[0] + assert y_np[2] == x[2] + alpha * x[1] + (alpha**2) * x[0] @pytest.mark.parametrize( argnames="mode", From cb4f7ddf9afde913cc5ec60248f4b71e2a9eb0ae Mon Sep 17 00:00:00 2001 From: Pablo deRoque Date: Thu, 22 Aug 2024 18:16:43 +0200 Subject: [PATCH 3/8] Add test_geometric_adstock_bad_alpha --- tests/mmm/test_transformers.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/mmm/test_transformers.py b/tests/mmm/test_transformers.py index c1454c324..8817509e2 100644 --- a/tests/mmm/test_transformers.py +++ b/tests/mmm/test_transformers.py @@ -18,6 +18,7 @@ import pytensor.tensor as pt import pytest import scipy as sp +from pymc.logprob.utils import ParameterValueError from pytensor.tensor.variable import TensorVariable from pymc_marketing.mmm.transformers import ( @@ -148,6 +149,21 @@ def test_geometric_adstock_good_alpha(self, x, alpha, l_max): assert y_np[1] == x[1] + alpha * x[0] assert y_np[2] == x[2] + alpha * x[1] + (alpha**2) * x[0] + @pytest.mark.parametrize( + "x, alpha, l_max", + [ + (np.ones(shape=(100)), -0.3, 10), + (np.ones(shape=(100)), -1.7, 100), + (np.ones(shape=(100)), 22.5, 7), + (np.linspace(start=0.0, stop=1.0, num=50), -0.8, 3), + (np.linspace(start=0.0, stop=1.0, num=50), 8, 50), + ], + ) + def test_geometric_adstock_bad_alpha(self, x, alpha, l_max): + with pytest.raises(ParameterValueError): + y = geometric_adstock(x=x, alpha=alpha, l_max=l_max) + y.eval() + @pytest.mark.parametrize( argnames="mode", argvalues=[ConvMode.After, ConvMode.Before, ConvMode.Overlap], From 9baa1d4bff974d144e2e46f9c4b87005f873a7cb Mon Sep 17 00:00:00 2001 From: Pablo deRoque Date: Thu, 22 Aug 2024 18:53:36 +0200 Subject: [PATCH 4/8] use ge, le instead of gt,lt --- pymc_marketing/mmm/transformers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pymc_marketing/mmm/transformers.py b/pymc_marketing/mmm/transformers.py index 49d35eed8..a8a6304c6 100644 --- a/pymc_marketing/mmm/transformers.py +++ b/pymc_marketing/mmm/transformers.py @@ -237,8 +237,8 @@ def geometric_adstock( """ alpha = check_parameters( - alpha, [pt.gt(alpha, 0), pt.lt(alpha, 1)], msg="0 <= alpha <= 1" - ).eval() + alpha, [pt.ge(alpha, 0), pt.le(alpha, 1)], msg="0 <= alpha <= 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 From fff06da3159e17c4c051b5726e4123f4f183298d Mon Sep 17 00:00:00 2001 From: Pablo de Roque Date: Thu, 22 Aug 2024 19:16:21 +0200 Subject: [PATCH 5/8] Update tests/mmm/test_transformers.py Co-authored-by: Will Dean <57733339+wd60622@users.noreply.github.com> --- tests/mmm/test_transformers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mmm/test_transformers.py b/tests/mmm/test_transformers.py index 8817509e2..3deb9ab03 100644 --- a/tests/mmm/test_transformers.py +++ b/tests/mmm/test_transformers.py @@ -160,8 +160,8 @@ def test_geometric_adstock_good_alpha(self, x, alpha, l_max): ], ) def test_geometric_adstock_bad_alpha(self, x, alpha, l_max): + y = geometric_adstock(x=x, alpha=alpha, l_max=l_max) with pytest.raises(ParameterValueError): - y = geometric_adstock(x=x, alpha=alpha, l_max=l_max) y.eval() @pytest.mark.parametrize( From aca40813e9fb71b2a57f873aedb6db3cf43f283b Mon Sep 17 00:00:00 2001 From: Pablo deRoque Date: Thu, 22 Aug 2024 19:22:56 +0200 Subject: [PATCH 6/8] Simplify test parameters --- tests/mmm/test_transformers.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/mmm/test_transformers.py b/tests/mmm/test_transformers.py index 3deb9ab03..10efb5f87 100644 --- a/tests/mmm/test_transformers.py +++ b/tests/mmm/test_transformers.py @@ -153,10 +153,9 @@ def test_geometric_adstock_good_alpha(self, x, alpha, l_max): "x, alpha, l_max", [ (np.ones(shape=(100)), -0.3, 10), - (np.ones(shape=(100)), -1.7, 100), - (np.ones(shape=(100)), 22.5, 7), - (np.linspace(start=0.0, stop=1.0, num=50), -0.8, 3), - (np.linspace(start=0.0, stop=1.0, num=50), 8, 50), + (np.ones(shape=(100)), -2, 10), + (np.ones(shape=(100)), 22.5, 10), + (np.ones(shape=(100)), 2, 10), ], ) def test_geometric_adstock_bad_alpha(self, x, alpha, l_max): From f3ce89fc1ac2c578f06e4e0d75d68f746ef3e371 Mon Sep 17 00:00:00 2001 From: Pablo de Roque Date: Thu, 22 Aug 2024 21:35:20 +0200 Subject: [PATCH 7/8] Update tests/mmm/test_transformers.py Co-authored-by: Will Dean <57733339+wd60622@users.noreply.github.com> --- tests/mmm/test_transformers.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/mmm/test_transformers.py b/tests/mmm/test_transformers.py index 10efb5f87..282a53f70 100644 --- a/tests/mmm/test_transformers.py +++ b/tests/mmm/test_transformers.py @@ -150,15 +150,13 @@ def test_geometric_adstock_good_alpha(self, x, alpha, l_max): assert y_np[2] == x[2] + alpha * x[1] + (alpha**2) * x[0] @pytest.mark.parametrize( - "x, alpha, l_max", - [ - (np.ones(shape=(100)), -0.3, 10), - (np.ones(shape=(100)), -2, 10), - (np.ones(shape=(100)), 22.5, 10), - (np.ones(shape=(100)), 2, 10), - ], + "alpha", + [-0.3, -2, 22.5, 2], + ids=["less_than_zero_0", "less_than_zero_1", "greater_than_one_0", "greater_than_one_1"], ) - def test_geometric_adstock_bad_alpha(self, x, alpha, l_max): + def test_geometric_adstock_bad_alpha(self, alpha): + l_max = 10 + x = np.ones(shape=100) y = geometric_adstock(x=x, alpha=alpha, l_max=l_max) with pytest.raises(ParameterValueError): y.eval() From f10a76f7a4c996c7150099649cc00e2960ff6541 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 22 Aug 2024 20:24:12 +0000 Subject: [PATCH 8/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/mmm/test_transformers.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/mmm/test_transformers.py b/tests/mmm/test_transformers.py index 282a53f70..232711a3f 100644 --- a/tests/mmm/test_transformers.py +++ b/tests/mmm/test_transformers.py @@ -152,7 +152,12 @@ def test_geometric_adstock_good_alpha(self, x, alpha, l_max): @pytest.mark.parametrize( "alpha", [-0.3, -2, 22.5, 2], - ids=["less_than_zero_0", "less_than_zero_1", "greater_than_one_0", "greater_than_one_1"], + ids=[ + "less_than_zero_0", + "less_than_zero_1", + "greater_than_one_0", + "greater_than_one_1", + ], ) def test_geometric_adstock_bad_alpha(self, alpha): l_max = 10