fix issue #32

[FMenchetti]

I added a function (i.e. 'model') to define the structural time series model without calling 'SSModel' explicitely. I'm opening a pull request so to promote discussion. @izahn @vliublinska , if you think that would be ok I can merge it, otherwise feel free to suggest modifications/enhancements.

I was thinking that maybe we can define only one function (e.g. 'MBSTSmodel' or whatever) that takes in the desired model components and priors, estimates the model and returns an 'mbsts' object. This function would replace both 'model' and 'mbsts.mcmc'; in this way, both 'predict.mbsts' and 'causal.mbsts' would be methods for objects of class 'mbsts', so that the user can easily choose to perform a proper causal analysis or just a prediction. In both instances, (s)he would just need to call 'MBSTSmodel' and either predict( ) or causal( ). What do you think?

[vliublinska]

I think the new function looks great! and I see where you are going with the suggestion to "merge" the new model() and mbsts.mcmc() so that the user doesn't need to run any intermediate steps. I wonder if we could go one step further and imbed model() and mbsts.mcmc() into predict...() and causal....() routines altogether, so that the structure of the model is fed into them directly (it can even be optional with some basic model fitted by default) along with other parameters necessary for mbsts.mcmc(). The latter can be a non-exported function.

We can still keep an option for the user to feed an SSModel object into predict...() and causal....() too, in case they want more flexibility in defining the model.

What do you guys think?

[FMenchetti]

If we feed model() and mbsts.mcmc() into predict(), the latter would look like the following predict(y, components, seas.period, cycle.period, s0.eps, s0.r, H, ....) and won't be a method for fitted objects anymore, while I'd like it to be more similar to predict.bsts, or even to forecast.lm in the forecast package.

Instead, we can certainly embed model() and mbsts.mcmc() into causal.mbsts, which might be renamed CausalMBSTS (main function that gives the name to the package, as it is done in CausalImpact). In this release I'd be cautious about default models, it really depends on the time series you have so it would be necessary to include some automated checks to identify model components (test for stationarity, seasonality...) and then feed them to model(). In the end, I think that would be great, but it's too much work for now (we were hoping to link the first release of the package to the submission of our paper in the next few weeks).

So, since you said the model function is ok :-) I'm merging this pull request and closing issue #32. I'l also update issue #33 with these latest suggestions.

[vliublinska]

@FMenchetti Got it! I've also been meaning to suggest to perhaps call the model() function something like as.SSModel just to make it a bit more clear what the function does. Sorry to keep dwelling on all these function names :-)

Update: Please, ignore if we are combining it with mbsts.mcmc!