LAMPSPUC / StateSpaceModels.jl

StateSpaceModels.jl is a Julia package for time-series analysis using state-space models.
https://lampspuc.github.io/StateSpaceModels.jl/latest/
MIT License
272 stars 25 forks source link

BasicStructuralExplanatory works ONLY for s = 12. #306

Closed junpei-n closed 2 years ago

junpei-n commented 2 years ago

BasicStructuralExplanatory works only for s=12 (seasonality). (on StateSpaceModels v0.6.1)

julia> BasicStructuralExplanatory(rand(100), 12, rand(100, 2))
BasicStructuralExplanatory

julia> BasicStructuralExplanatory(rand(100), 10, rand(100, 2))
ERROR: ArgumentError: number of rows of each array must match (got (8, 8, 8, 10))

So, I fixed following code to work for other seasonality.

function BasicStructuralExplanatory(y::Vector{Fl}, s::Int, X::Matrix{Fl}) where Fl
...
        Z = [vcat([1; 0; 1; zeros(Fl, s - 2)], X[t, :]) for t in 1:num_observations]
        T = [[
            1 1 zeros(Fl, 1, s - 1) zeros(Fl, 1, num_exogenous)
            0 1 zeros(Fl, 1, s - 1) zeros(Fl, 1, num_exogenous)
            0 0 -ones(Fl, 1, s - 1) zeros(Fl, 1, num_exogenous)
            zeros(Fl, s - 2, 2) Matrix{Fl}(I, s - 2, s - 2) zeros(Fl, s - 2) zeros(Fl, s - 2, num_exogenous)  # last was zeros(Fl, 10, num_exogenous)
            zeros(Fl, num_exogenous, s + 1) Matrix{Fl}(I, num_exogenous, num_exogenous)  # first was zeros(Fl, num_exogenous, 13)
        ] for _ in 1:num_observations]
...

If this fix is right, I hope the code will be fixed.

guilhermebodin commented 2 years ago

Hi @junpei-n thank you for pointing that out! I am opening a PR with you suggestion

guilhermebodin commented 2 years ago

It will be release on version 0.6.2

junpei-n commented 2 years ago

It works now. Thank you for your quick response, @guilhermebodin !