Repository navigation
fix: cap the auto yearly seasonality order at what the sampling rate supports - #2774
Open
NishchayMahor wants to merge 2 commits into
Open
NishchayMahor wants to merge 2 commits into
NishchayMahor wants to merge 2 commits into
Conversation
…supports A seasonality is only identifiable up to the Nyquist limit of the spacing between observations. Monthly data carries 12 points a year, so 6 harmonics can be resolved, but the auto order was 10 regardless of frequency. The top four harmonics alias onto lower ones and their coefficients are set by the prior rather than by the data, so the fitted shape moves a long way for a small change in the input. On the series in meta#2677, shifting the same 48 monthly values by one calendar year moved the 12-month forecast total by -25.1% and one point by -104.5%, flipping it negative because the multiplicative term reached -1.05. With the order capped to 6 the same shift moves the total by -0.3% and nothing goes negative. Weekly and daily seasonality already decline to turn on once the spacing reaches their period. This applies the same reasoning to yearly, keeping the component and lowering the order so monthly data still gets a yearly shape. Hourly, daily and weekly sampling are unaffected, and an explicitly requested order is left alone.
The weekly and daily guards already carry this suppression for the same false positive on min_dt.
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.
Fixes #2677.
In that issue the reporter takes a 48-month series, shifts every date forward by one
year, leaves
yuntouched, and the 12-month forecast total moves by -25%. Onepoint flips sign. @Fediuld found that
yearly_seasonality=Falsemakes it go away, and@gboulmier then asked the question this PR answers: why would turning off a seasonality
the data clearly has make the fit more stable?
Why it happens
Monthly data has one sample per month, so a yearly cycle is sampled 12 times. Above the
Nyquist limit of that spacing,
floor(365.25 / (2 * 30.4))= 6 harmonics, the higherharmonics alias onto lower ones. Their coefficients are then set by the prior rather
than by the data, so the fitted shape can move a long way for a small change in the
input.
yearly_seasonality='auto'asks for order 10 regardless of spacing, so four ofthose ten harmonics are unidentifiable.
Prophet already guards the other two seasonalities on exactly this basis.
forecaster.pydeclines weekly once
min_dt >= Timedelta(weeks=1)and daily oncemin_dt >= Timedelta(days=1). Yearly has nomin_dtterm at all. That inconsistency isthe argument here, not a general claim about what users do.
The change
set_auto_seasonalitiescaps the default yearly order at the Nyquist limit of theobserved minimum spacing. Weekly and daily drop the component entirely at that point;
yearly keeps the component and lowers the order instead, so monthly data still gets a
yearly shape. An explicitly requested order is left alone.
Effect on the reported series
Reporter's own data, dates shifted one year,
yunchanged,uncertainty_samples=0:The multiplicative term below -1 is what produced the sign flip:
yhat = trend * (1 + m).Test suite: 139 passed, 6 skipped, against a 138/6 baseline I measured on
5cf15c0.The new test asserts the order for six frequencies plus the two override paths, and
fails on current main.
What I did not verify, and the tradeoffs
min_dtis the minimum gap, not the typical one. A monthly series with one stray1-day gap keeps order 10 and is unhelped. I verified that. It is the fail-safe
direction and it matches how the weekly and daily guards already behave, but median
spacing would bind more often if you would rather have that.
autowith monthly data gets a differentyearly shape after this. An
logger.infosays so and names the override. If you wouldrather not move existing fits, the warning alone (no cap) is a one-line version of this
PR and I am happy to cut it back to that.
is identifiability and internal consistency, not that forecasts get better on average.
data. I left the docs alone pending a call on the approach.