Add WeeklyFourier #1443

PabloRoque · 2025-01-28T10:44:17Z

Description

Introduces WeeklyFourier
Fixes sample_curve. Now returns same sizes when called with use_dates: bool = True | False [see test_sample_curve_same_size]
Parametrizes some tests so they are now run on the 3 different Fourier classes.

Related Issue

Closes #
Related to Fourier seasonality transformations #1442

Checklist

Checked that the pre-commit linting/style checks pass. Feel free to comment pre-commit.ci autofix to auto-fix.
Included tests that prove the fix is effective or that the new feature works
Added necessary documentation (docstrings and/or example notebooks) using numpydoc format.
If you are a pro: each commit corresponds to a relevant logical change

📚 Documentation preview 📚: https://pymc-marketing--1443.org.readthedocs.build/en/1443/

codecov · 2025-01-28T10:49:25Z

Codecov Report

Attention: Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.

Project coverage is 94.61%. Comparing base (dd2ff1f) to head (02ca97b).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
pymc_marketing/mmm/fourier.py	95.23%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1443      +/-   ##
==========================================
- Coverage   94.61%   94.61%   -0.01%     
==========================================
  Files          50       50              
  Lines        5423     5441      +18     
==========================================
+ Hits         5131     5148      +17     
- Misses        292      293       +1

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

wd60622 · 2025-01-28T12:33:13Z

pymc_marketing/mmm/fourier.py

@@ -833,6 +833,63 @@ def _get_default_start_date(self) -> datetime.datetime:
        return datetime.datetime(year=now.year, month=now.month, day=1)


+class WeeklyFourier(FourierBase):
+    """Monthly fourier seasonality.


My bad, missed this one

wd60622 · 2025-01-28T12:33:45Z

pymc_marketing/mmm/fourier.py

+    """Monthly fourier seasonality.
+
+    .. plot::
+        :context: close-figs


Think a space is needed here. It is not rendering in the docs for some reason

Wrong import + missing from __init__.py. Should be fixed now (tested locally)

wd60622 · 2025-01-28T12:34:07Z

pymc_marketing/mmm/fourier.py

+    days_in_period: float = DAYS_IN_WEEK
+
+    def _get_default_start_date(self) -> datetime.datetime:
+        """Get the default start date for monthly seasonality.


Docstring change

PabloRoque · 2025-01-29T15:48:54Z

@wd60622 Anything else on this PR?

wd60622 · 2025-01-29T15:51:33Z

@wd60622 Anything else on this PR?

Is the doc rendering correctly?

wd60622

Looks good. Just a request to deduplicate tests

tests/mmm/test_fourier.py

wd60622 · 2025-01-29T15:54:09Z

tests/mmm/test_fourier.py

+@pytest.mark.parametrize(
+    argnames="seasonality",
+    argvalues=[YearlyFourier, MonthlyFourier, WeeklyFourier],
+    ids=[
+        "yearly",
+        "monthly",
+        "weekly",
+    ],
+)
+def test_sample_curve_same_size(seasonality) -> None:
+    n_order = 2
+    periodicity = seasonality(n_order=n_order)
+    prior = periodicity.sample_prior(samples=10)
+    curve_without_dates = periodicity.sample_curve(prior, use_dates=False)
+    curve_with_dates = periodicity.sample_curve(prior, use_dates=False)
+
+    assert curve_without_dates.sizes == curve_with_dates.sizes


Not needed the tests as above are same

This is to test that the changes I made to apply make sense. This test was not passing before.

wd60622 · 2025-01-29T16:07:13Z

tests/mmm/test_fourier.py

+    periodicity = seasonality(n_order=n_order)
+    prior = periodicity.sample_prior(samples=10)
+    curve_without_dates = periodicity.sample_curve(prior, use_dates=False)
+    curve_with_dates = periodicity.sample_curve(prior, use_dates=False)


Why is this false as well? Or is the variable name wrong?

doh!

The sample_curve(..., use_dates) param took both values in my mind. Fixed now

The introduction of np.ceil in sample_curve still holds

PabloRoque · 2025-01-29T18:23:08Z

@wd60622 Anything else on this PR?

Is the doc rendering correctly?

Yes. See docs build

wd60622 · 2025-01-29T20:25:26Z

tests/mmm/test_fourier.py

+def test_sample_curve_same_size(seasonality) -> None:
+    n_order = 2
+    periodicity = seasonality(n_order=n_order)
+    prior = periodicity.sample_prior(samples=10)
+    curve_without_dates = periodicity.sample_curve(prior, use_dates=False)
+    curve_with_dates = periodicity.sample_curve(prior, use_dates=True)
+
+    assert curve_without_dates.shape == curve_with_dates.shape


Still dont understand how this is not duplicate but thats fine

Add WeeklyFourier and related tests

948d094

github-actions bot added MMM tests labels Jan 28, 2025

wd60622 added the model components Related to the various model components label Jan 28, 2025

wd60622 reviewed Jan 28, 2025

View reviewed changes

PabloRoque added 3 commits January 28, 2025 14:50

Fix sample_curve to return equivalent sizes

b1737d0

Fix docstring

7cd3d53

Fix docstring

5d7b1e4

PabloRoque changed the title ~~Add WeeklyFourier and related tests~~ Add WeeklyFourier Jan 28, 2025

Fix docs

d704553

PabloRoque self-assigned this Jan 28, 2025

Add WeeklyFourier to __init__.py

ac0515f

PabloRoque marked this pull request as ready for review January 28, 2025 16:17

PabloRoque requested a review from wd60622 January 28, 2025 16:32

Merge branch 'main' into fourier-seasonalities

ab863ce

wd60622 approved these changes Jan 29, 2025

View reviewed changes

wd60622 requested changes Jan 29, 2025

View reviewed changes

PabloRoque added 3 commits January 29, 2025 18:12

Parametrize tests

748ce71

Fix test_sample_curve_same_size

da1b1be

Revert some parametrizations

02ca97b

PabloRoque requested a review from wd60622 January 29, 2025 18:20

wd60622 approved these changes Jan 29, 2025

View reviewed changes

wd60622 merged commit 759bfd2 into pymc-labs:main Jan 29, 2025
20 checks passed

PabloRoque deleted the fourier-seasonalities branch January 29, 2025 21:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add WeeklyFourier #1443

Add WeeklyFourier #1443

PabloRoque commented Jan 28, 2025 •

edited

Loading

codecov bot commented Jan 28, 2025 •

edited

Loading

wd60622 Jan 28, 2025

PabloRoque Jan 28, 2025

wd60622 Jan 28, 2025

PabloRoque Jan 28, 2025

wd60622 Jan 28, 2025

PabloRoque Jan 28, 2025

PabloRoque commented Jan 29, 2025

wd60622 commented Jan 29, 2025

wd60622 left a comment

wd60622 Jan 29, 2025

PabloRoque Jan 29, 2025

wd60622 Jan 29, 2025

PabloRoque Jan 29, 2025

PabloRoque commented Jan 29, 2025

wd60622 Jan 29, 2025

Add WeeklyFourier #1443

Add WeeklyFourier #1443

Conversation

PabloRoque commented Jan 28, 2025 • edited Loading

Description

Related Issue

Checklist

codecov bot commented Jan 28, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PabloRoque commented Jan 29, 2025

wd60622 commented Jan 29, 2025

wd60622 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PabloRoque commented Jan 29, 2025

Choose a reason for hiding this comment

PabloRoque commented Jan 28, 2025 •

edited

Loading

codecov bot commented Jan 28, 2025 •

edited

Loading