mitchelloharawild / fable.prophet

fable extension for the prophet forecasting procedure
https://pkg.mitchelloharawild.com/fable.prophet
55 stars 8 forks source link

Issue with using numeric names for seasonality #9

Closed mitchelloharawild closed 5 years ago

mitchelloharawild commented 5 years ago

From @mpjashby

If I change the model() function in the code above to:

prophet_models <- model(
    daily_crimes,
    prophet_year = prophet(n ~ growth() + season(period = "year")),
    prophet_365 = prophet(n ~ growth() + season(period = 365)),
    prophet_365_order = prophet(n ~ growth() + season(period = 365, order = 4)),
    prophet_week = prophet(n ~ growth() + season(period = "week"))
)

I get:

1: 1 error encountered for prophet_year
[1] invalid class “Period” object: periods must have integer values

2: 1 error encountered for prophet_365
[1] Unable to add season(period = 365) to the model. The fourier order has no 
    default, and must be specified with `order = ?`.

3: 1 error encountered for prophet_365_order
[1] Can't find column `365` in `.data`.

So I suppose I have two questions:

  1. how can I get season(period = "year") to work for daily data, and
  2. why is 365 in season(period = 365, order = ?) treated as a column name rather than a number?

I'm primarily interested in the first question, because if season(period = "year") works then I suppose there is probably little need for season(period = 365, order = ?)!

mitchelloharawild commented 5 years ago

Thanks for the issues.

The error for prophet_365 is as discussed earlier, there is no default order for period=365, and so you will have to specify it as the error suggests.

The errors for prophet_year and prophet_365_order are bugs introduced in the transition from Python to R verions of prophet. They should now be fixed.

mitchelloharawild commented 5 years ago

As for your first question, season(period = "year") works because it will correctly identify period = 365.25, and would be equivalent to season(period = 365.25).

The second question is more of a bug, but prophet requires each seasonal period to be given a name (not sure why this is required, it doesn't impact the model in any way). As the naming is mostly unimportant, fable.prophet attempts to give a name automatically. The bug is that in prophet's python, a name of "365" is perfectly valid, but in R the name will be converted to "X365" by prophet, resulting in an error.

mpjashby commented 5 years ago

They all now work on my machine – thanks very much for fixing these so quickly! The R interface for prophet seems very Pythonesque, so thanks for ironing out all the issues that come up in making it more R-like for fable.prophet.

mitchelloharawild commented 5 years ago

Great, thanks again for submitting the bugs!