baggepinnen / ControlSystemIdentification.jl

System Identification toolbox, compatible with ControlSystems.jl
https://baggepinnen.github.io/ControlSystemIdentification.jl/dev
MIT License
132 stars 13 forks source link

Regarding ar() #62

Closed mapi1 closed 3 years ago

mapi1 commented 3 years ago

I was just preparing some tests for the generalized least squares method, when I found that the by ar() returned tf is, as far as I can see it, incorrect. For example when na = 1 it has the structure 1 / (z - a1), but shouldn't it be z / (z - a1)? The origin is probably missing zeros in params2poly, but correct me if I am mistaken on this one.

baggepinnen commented 3 years ago

I think it depends on how you define the noise process. If you consider the model

y(t+1) = a y(t) + e(t)
y(t+1) - a y(t) = e(t)
(z - a) Y(z) = E(z)

then the model is

1/(z-a)

The alternative model would be

y(t+1) = a y(t) + e(t+1)

which would give you the transfer function

z/(z-a)

Since the default used to be no direct term, the same convention was used for ar. In practice, it matter less for ar since the noise process is not observed nor is it an input to the function.

mapi1 commented 3 years ago

Good point, it all comes down to the definition.

Some practical aspects I see is, that for the second definition the last statement is true,

N = 10000
y = zeros(N)
y[1] = randn()
u = copy(y)
for i in 2:N 
    y[i] = 0.9y[i-1]
end
Gar = tf([1, 0], [1, -0.9], 1)
y2 = lsim(Gar, u, 1:N)[1][:]
all(y .== y2)

whereas the first version gives a shifted result depending on na. But more important, one can use

u2 = lsim(1/Gar, y2, 1:N)[1][:] 
all(u .== u2)

to retain input noise process, the first definition results in an error here, as 1/Gar is improper there.

How do you see these aspects? Anyway I would write sth. about this in the docs of ar(), for slow folks like myself :D

baggepinnen commented 3 years ago

Those are both very desirable properties, I say we go with the updated definition. This example would be very nice to have in the Docstring. Documenting and being explicit about this choice helps both the user and future me 😊

baggepinnen commented 3 years ago

Closed by #64