search

tidyverts / fable

Tidy time series forecasting
https://fable.tidyverts.org
GNU General Public License v3.0
559 stars 65 forks source link

`generate.AR` is wrong because `AR` calculates `reg_resid` incorrectly #393

Closed FinYang closed 1 year ago

FinYang commented 1 year ago

I first noticed this when I tried to simulate from tsibbledata::pelt with AR(2) and got weird values. 

library(fable)
#> Loading required package: fabletools
tsibbledata::pelt %>% 
  model(AR(Lynx ~ order(2))) %>% 
  generate()
#> # A tsibble: 2 x 4 [1Y]
#> # Key:       .model, .rep [1]
#>   .model              .rep   Year        .sim
#>   <chr>               <chr> <dbl>       <dbl>
#> 1 AR(Lynx ~ order(2)) 1      1936 -125836108.
#> 2 AR(Lynx ~ order(2)) 1      1937  -15925364.

Created on 2023-06-20 with reprex v2.0.2

I reckon this bug might be introduced from the beginning when generate.AR is added. I didn't put much time into this but if I'm understanding it correctly, reg_resid is the remainder after removing the effect of linear predictors, specifically the intercept in AR, then there is no need to *x_sd here and x - xm should be fine.

https://github.com/tidyverts/fable/blob/9b926f8be993594c3ae448b912366aca09ab78d7/R/ar.R#L206