IQSS / Zelig

A statistical framework that serves as a common interface to a large range of models
http://zeligproject.org
110 stars 43 forks source link

sim method returns error for AR and MA models #287

Closed mbsabath closed 7 years ago

mbsabath commented 7 years ago

going through the examples in the arima vignette, except with AR and MA models, as well as the order parameter remove returns this error when ts.out$sim() is run:

Error in envRefInferField(x, what, getClass(class(x)), selfEnv) : 
  ‘qi’ is not a valid field or method name for reference class “Zelig-ma”
christophergandrud commented 7 years ago

@tercer do you know the status of sim for AR and MA models? Was this implemented?

mbsabath commented 7 years ago

I did a bit of digging into the issue on a local branch, the issue looks like it has two parts. First, AR and MA only inherit from timeseries, but not from ARIMA, and ARIMA has the qi method causing the initial issue.

However, even when they do inherit from ARIMA, there are still issues. sim doesn't initially throw an error when it's called. But when summary(ts.out) is called after running sim, an error is thrown indicating that qi is an empty matrix. I haven't been able to find the source of that issue yet.

mbsabath commented 7 years ago

Doing a bit more digging into this, it looks like the sim method is also not fully implemented for ARIMA models.

christophergandrud commented 7 years ago

Really good digging. I wouldn't be surprised if it hasn't been fully implemented, qi's for timeseries are really tricky.

At the least in the meantime the docs should flag this issue.

On Fri, Jun 30, 2017 at 5:43 PM Ben Sabath notifications@github.com wrote:

Doing a bit more digging into this, it looks like the sim method is also not fully implemented for ARIMA models.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/IQSS/Zelig/issues/287#issuecomment-312302189, or mute the thread https://github.com/notifications/unsubscribe-auth/ABOerURbw75WNSrR0vz7WnN4aefALHEYks5sJRelgaJpZM4OCwZd .

mbsabath commented 7 years ago

Sounds good, I made the changes, and am going to push them up to GitHub soon.

mbsabath commented 7 years ago

After further investigation, fixing the inheritance issue allows plot to work for the ar and ma methods. The errors that occur after calling summary are still consistent for all of the time series models, and require further investigation.

Adding the syntax that causes the error:

ts.out <- zelig(x~y, model = ('ar' or 'ma' or 'arima'), data = data)
setx(ts.out, value)
setx1(ts.out, value1)
sim(ts.out)
summary(ts.out)

The reference class syntax also causes the error. After the call to summary, this output is returned:

 sim x :
 -----
ev

Error in apply(qi, 2, quantile, c(0.5, 0.025, 0.975), na.rm = TRUE) : 
  dim(X) must have a positive length

As a comparison, the output from a similar code pattern, but using the logic model returns this output as a post simulation summary:

 sim x :
 -----
ev
          mean         sd       50%      2.5%     97.5%
[1,] 0.7652166 0.01010116 0.7653829 0.7450536 0.7846447
pv
         0     1
[1,] 0.257 0.743
christophergandrud commented 7 years ago

I've taken the ar and ma models out of the documentation (still needs to filter through the website). They were just (very buggy) wrappers for arima. Until they are actually working well, we shouldn't publicise them.

https://github.com/IQSS/Zelig/commit/d36713e3fb1e65672fb98396174f00db61caba45

christophergandrud commented 7 years ago

Going with arima overall syntax. summary issue reported separately in https://github.com/IQSS/Zelig/issues/305