Skip to content

Index the per-season alpha in the seasonal ES models - #1226

Open
Venish Paneliya (VenishPaneliya) wants to merge 1 commit into
Nixtla:mainfrom
VenishPaneliya:seasonal-es-per-season-alpha
Open

Venish Paneliya (VenishPaneliya) wants to merge 1 commit into
Nixtla:mainfrom
VenishPaneliya:seasonal-es-per-season-alpha

Conversation

@VenishPaneliya

Copy link
Copy Markdown
Contributor

SeasonalExponentialSmoothing and SeasonalExponentialSmoothingOptimized fit one alpha per season, so model_["alpha"] has shape (season_length,). Two places were copied from the scalar SimpleExponentialSmoothing and use it unindexed, so both raise:

m = SeasonalExponentialSmoothing(season_length=12, alpha=0.1)
m.fit(y)

m.predict(h=6, level=[80])
# ValueError: operands could not be broadcast together with shapes (6,) (12,)

m.simulate(h=6, n_paths=4)
# ValueError: operands could not be broadcast together with shapes (12,) (4,)

Both variants are affected; the scalar SimpleExponentialSmoothing works, and the only difference is the shape of alpha:

SeasonalExponentialSmoothing            alpha shape (12,)   predict(level) raises   simulate raises
SeasonalExponentialSmoothingOptimized   alpha shape (12,)   predict(level) raises   simulate raises
SimpleExponentialSmoothing              alpha shape ()      predict(level) OK       simulate OK

For predict, the interval needs the alpha of the season each step falls in. k = ((steps - 1) // m) + 1 on the line above already gives the cycle number, so the season index is (steps - 1) % m.

For simulate, s_idx = i % m is already computed on the previous line for levels, and alpha needs the same index.

Scope:

  • The scalar models are untouched. Their alpha is 0-d and their equivalent lines read differently (levels = alpha * ..., no [:, s_idx]), so they cannot be caught by this change — verified their predict(level=...) and simulate output is identical before and after.
  • Point forecasts are identical before and after for all four models. Only the two paths that raised now return values.
  • tests/test_models.py is 204 passed both before and after this change, since nothing that previously worked is touched.

Added a parametrised test covering both variants: it checks predict(h, level=[80]) returns ordered bounds and that simulate returns the expected shape. It fails on main for both and passes here.

Worth noting these paths appear never to have worked — the seasonal predict(level=...) and simulate have raised since they were added.

`SeasonalExponentialSmoothing` and its optimized variant fit one alpha
per season, so `model_["alpha"]` has shape `(season_length,)`. Two places
copied from the scalar `SimpleExponentialSmoothing` use it unindexed, and
both raise:

    SeasonalExponentialSmoothing(season_length=12, alpha=0.1).fit(y)
    .predict(h=6, level=[80])
    ValueError: operands could not be broadcast together with shapes (6,) (12,)

    .simulate(h=6, n_paths=4)
    ValueError: operands could not be broadcast together with shapes (12,) (4,)

`predict` needs the alpha of the season each step falls in, and the
existing `k = ((steps - 1) // m) + 1` already gives the cycle, so the
season is `(steps - 1) % m`. `simulate` already computes `s_idx = i % m`
for `levels` on the line above and needs the same index for `alpha`.

The scalar models are untouched - their `alpha` is 0-d and their two
call sites read differently, so nothing that currently works changes.
Point forecasts are identical before and after; only the paths that
raised now return values.
@codspeed

codspeed Bot commented Aug 31, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 38 untouched benchmarks


Comparing VenishPaneliya:seasonal-es-per-season-alpha (43f427b) with main (8b28576)

Open in CodSpeed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant