Skip to content

fix: reject a non-positive add_seasonality period - #2775

Open
kevin9327 wants to merge 1 commit into
meta:mainfrom
kevin9327:fix/add-seasonality-period
Open

kevin9327 wants to merge 1 commit into
meta:mainfrom
kevin9327:fix/add-seasonality-period

Conversation

@kevin9327

Copy link
Copy Markdown

add_seasonality validates fourier_order and prior_scale, but not period. A zero or NaN period is accepted, and the failure only shows up later in fit, far from the call that caused it:

m = Prophet()
m.add_seasonality(name="x", period=0, fourier_order=3)   # no error
m.fit(df)
# ZeroDivisionError: division by zero   (fourier_series: (i + 1) / period)

With period=float("nan") the seasonality features are all NaN and fit fails with RuntimeError: Error during optimization! from cmdstan, with nothing pointing at the seasonality.

period is the length of the cycle in days and divides the Fourier frequencies, so it has to be positive.

Change

add_seasonality now raises ValueError('Period must be > 0') next to the existing fourier_order check. The check is written as not period > 0 so it also catches NaN. Nothing is stored when it raises.

Tests

New test_custom_period next to test_custom_fourier_order: period of 0, 0.0, -7 and nan each raise, and the seasonality is not added. It fails before the change and passes after. python/prophet/tests/test_prophet.py: 63 passed, 2 skipped (run from source with the compiled Stan model from the 1.x wheel).

This change was prepared with AI assistance; I reproduced the failures above and ran the tests.

add_seasonality checked fourier_order and prior_scale but not period. A
period of 0 was accepted and fit then failed with a bare
ZeroDivisionError from fourier_series; a NaN period was accepted and
the Stan optimizer failed. Raise ValueError('Period must be > 0') when
the seasonality is added.
@meta-cla meta-cla Bot added the CLA Signed label Oct 31, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant