Repository navigation
Index the per-season alpha in the seasonal ES models - #1226
Open
Venish Paneliya (VenishPaneliya) wants to merge 1 commit into
Open
Venish Paneliya (VenishPaneliya) wants to merge 1 commit into
Venish Paneliya (VenishPaneliya) wants to merge 1 commit into
Conversation
`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.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SeasonalExponentialSmoothingandSeasonalExponentialSmoothingOptimizedfit one alpha per season, somodel_["alpha"]has shape(season_length,). Two places were copied from the scalarSimpleExponentialSmoothingand use it unindexed, so both raise:Both variants are affected; the scalar
SimpleExponentialSmoothingworks, and the only difference is the shape ofalpha:For
predict, the interval needs the alpha of the season each step falls in.k = ((steps - 1) // m) + 1on the line above already gives the cycle number, so the season index is(steps - 1) % m.For
simulate,s_idx = i % mis already computed on the previous line forlevels, andalphaneeds the same index.Scope:
alphais 0-d and their equivalent lines read differently (levels = alpha * ..., no[:, s_idx]), so they cannot be caught by this change — verified theirpredict(level=...)andsimulateoutput is identical before and after.tests/test_models.pyis 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 thatsimulatereturns the expected shape. It fails onmainfor both and passes here.Worth noting these paths appear never to have worked — the seasonal
predict(level=...)andsimulatehave raised since they were added.