Closed peroveh closed 3 years ago
@mapi1 do you think this is related to https://github.com/baggepinnen/ControlSystemIdentification.jl/commit/890a3ee9e022055a136c1128146846eb402b6e14 or https://github.com/baggepinnen/ControlSystemIdentification.jl/commit/2d212e3106670708092bd4bf50c09cdc6314db8d ?
Here's some code to roughly reproduce what's seen above since no code was posted
using ControlSystemIdentification, ControlSystems, Plots
using Random, LinearAlgebra
default(size=(500,280))
N = 500 # Number of time steps
t = 1:N
Δt = 1 # Sample time
u = randn(N) # A random control input
G = tf(0.8, [1,0,0.25], 1) # An interesting system
sim(sys,u) = lsim(sys, u, t)[1][:]
y = sim(G,u)
yn = y + randn(size(y));
# Validation data
uv = randn(N)
yv = sim(G,uv)
ynv = yv + randn(size(yv));
na,nb = 2,1 # Number of polynomial coefficients to estimate
d = iddata(yn,u,Δt)
Gls = arx(d,na,nb)
Chaning the inputdelay
parameter does recover a better transfer function (but does not change the numerator order as would be expected)
julia> Gls = arx(d,2,1)
TransferFunction{Discrete{Int64}, SisoRational{Float64}}
-0.053195176045275815
----------------------------------------------------
1.0z^2 + 0.020944097150896568z + 0.13061559216712304
Sample Time: 1 (seconds)
Discrete-time transfer function model
julia> Gls = arx(d,2,1, inputdelay=2)
TransferFunction{Discrete{Int64}, SisoRational{Float64}}
0.8711130327440996
---------------------------------------------------
1.0z^2 - 0.008279348253820712z + 0.1078308656207146
Sample Time: 1 (seconds)
Discrete-time transfer function model
@peroveh note that arx
is expected to perform poorly since what you are simulating is an output-error process but you're estimating an input-error model. They should not perform this poorly though, and I believe it's related to an internal index shift that has gone wrong.
I was surprised that the error was low if na=nb=2 in the solver, but worse when na=2 and nb=1 (which should be sufficient)
Btw: is n the order of the largest z term, or is it the number of coefficients...? I.e. for numerator 1.1z^2+0.8z+0.1 should this require nb=2 or 3?
Den 14. august 2021 kl. 16.42.25 +02.00 skrev Fredrik Bagge Carlson @.***>:
@mapi1 https://github.com/mapi1 do you think this is related to 890a3ee
or 2d212e3 ?
Here's some code to roughly reproduce what's seen above since no code was posted
using ControlSystemIdentification, ControlSystems, Plots
using Random, LinearAlgebra
default(size=(500,280))
N = 500 # Number of time steps
t = 1:N
Δt = 1 # Sample time
u = randn(N) # A random control input
G = tf(0.8, [1,0,0.25], 1) # An interesting system
sim(sys,u) = lsim(sys, u, t)[1][:]
y = sim(G,u)
yn = y + randn(size(y));
Validation data
uv = randn(N)
yv = sim(G,uv)
ynv = yv + randn(size(yv));
na,nb = 2,1 # Number of polynomial coefficients to estimate
d = iddata(yn,u,Δt)
Gls = arx(d,na,nb) Chaning the inputdelay parameter does recover a better transfer function julia> Gls = arx(d,2,1)
TransferFunction{Discrete{Int64}, SisoRational{Float64}}
-0.053195176045275815
1.0z^2 + 0.020944097150896568z + 0.13061559216712304
Sample Time: 1 (seconds)
Discrete-time transfer function model
julia> Gls = arx(d,2,1, inputdelay=2)
TransferFunction{Discrete{Int64}, SisoRational{Float64}}
0.8711130327440996
1.0z^2 - 0.008279348253820712z + 0.1078308656207146
Sample Time: 1 (seconds)
Discrete-time transfer function model @peroveh https://github.com/peroveh note that arx is expected to perform poorly since what you are simulating is an output-error process but you're estimating an input-error model. They should not perform this poorly though, and I believe it's related to an internal index shift that has gone wrong.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/baggepinnen/ControlSystemIdentification.jl/issues/75#issuecomment-898902872, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD7ALMGX2X6SVTBEEVMGWCLT4Z6FDANCNFSM5CAIMU5A. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.
I don't think it is particularly related, but it probably made things more complicated.
I also think this behavior is correct (though probably not well enough documented). To clarify this a little bit: nb
is the number of coefficients, while the order is given by nb + inputdelay - 1
, which also differs from the definition used by eg. Matlab potentially confusing users coming from there.
So for the example above:
G = tf(0.8, [1,0,0.25], 1)
TransferFunction{Discrete{Int64}, ControlSystems.SisoRational{Float64}} 0.8 ------------- 1.0z^2 + 0.25
nb = 1
is enough with respect to the number of coefficients but as the order ofB
is 2, the estimation still fails. Settinginputdelay = 2
fixes this while maintaining the number of coefficients.
Maybe it would help to print something like the following in order to make clear what is actually estimated?
julia> Gls = arx(d,2,1)
┌ Info: Estimating TransferFunction of form:
│ b₁z^1
│ -------------------
└ 1.0z^2 + a₁z^1 + a₂
...
julia> Gls = arx(d,2,1,inputdelay = 2)
┌ Info: Estimating TransferFunction of form:
│ b₁
│ -------------------
└ 1.0z^2 + a₁z^1 + a₂
...
There's probably still an error though, as the z^1
in your printout is not reflected in the actual transfer functions returned by the estimation
julia> Gls = arx(d,2,1)
-0.053195176045275815
----------------------------------------------------
1.0z^2 + 0.020944097150896568z + 0.13061559216712304
julia> Gls = arx(d,2,1, inputdelay=2)
0.8711130327440996
---------------------------------------------------
1.0z^2 - 0.008279348253820712z + 0.1078308656207146
no matter how the inputdelay
is defined, there should be a difference in the order of the z
in the numerator here?
You are right, there is an error. The case na > nb, (or better order of A > order of B) is missing in params2poly2 and from the tests. I will open a PR fixing this and trying to be more clear in the documentation.
Apart from that, I found something that is strange, this little snippet passes as true when I add it to the tests, though it shouldn't right? What am I missing?
G1 = tf([0.8], [1,0,0.25],
G2 = tf([0.8, 0], [1,0,0.25], 1)
@test isapprox(G1, G2)
I will open a PR fixing this and trying to be more clear in the documentation.
Very much appreciated!
though it shouldn't right?
You're right, that appears to be a bug in ControlSystems.jl
Thanks to @mapi1, this issue is now resolved on the master branch of this package. To get your desired behavior, you must supply the correct value to the inputdelay
keyword argument, e.g.,
julia> Gls = arx(d,na,nb, inputdelay=2)
TransferFunction{Discrete{Int64}, ControlSystems.SisoRational{Float64}}
0.8368547377892526
----------------------------------------------------
1.0z^2 - 0.002375293735401682z + 0.07787696411618848
Hi Fredrik I am a bit uncertain about these inputs.. Given that na is the (maximum?) order of the denominator, nb is of the numerator,,, what is the extra inputdelay? Will this be the max of na and nb? Or is the model a concatenation of a arma and a fixed delay of two timesteps? Are there any docomentation to look at for usage and algorithmic description?
Per Oe On August 20, 2021 at 2:15:17 pm +02:00, Fredrik Bagge Carlson @.***> wrote:
Thanks to @mapi1 https://github.com/mapi1, this issue is now resolved on the master branch of this package. To get your desired behavior, you must supply the correct value to the inputdelay keyword argument, e.g.,
julia> Gls = arx(d,na,nb, inputdelay=2) TransferFunction{Discrete{Int64}, ControlSystems.SisoRational{Float64}} 0.8368547377892526
1.0z^2 - 0.002375293735401682z + 0.07787696411618848
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/baggepinnen/ControlSystemIdentification.jl/issues/75#issuecomment-902649637, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD7ALMEOPQP4V7QLDTXKNE3T5ZBNLANCNFSM5CAIMU5A. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.
Thanks to @mapi1, this issue is now resolved on the master branch of this package. To get your desired behavior, you must supply the correct value to the
inputdelay
keyword argument, e.g., How do I ensure that i get the corrected vesion in the master branch? Can I specify a specific version of ControlSystems.jl or do I need to download the latest Julia release?
To install the latest version you can just use ]
+ add ControlSystemIdentification#master
from the REPL.
In the latest version, the documentation help?> arx
is then also updated which should answer some of your questions.
Using the master branch will also require using the master branch of ControlSystems, as there has been some yet-to-be-released breaking changes.
Apparent issues with several of the arx() estimators used in your identification_arx.ipynb example with slightly modified AR and ARMA transfer functions with the following solvers: (I assume they all should handle the MA part also given nb is specified? I did not find an armax version)
1) Tested pure AR with two imaginary poles on
A small error is added to simulated data
Testing various specifications of system order (denominator and numerator). I am a bit uncertain if n is the number of coefficients here or the order?
With na=2,nb=2,nc=1 (?) they all gave reasonably similar and ok results..
With na=2, nb=1 (should suffice for the numerator) the arx versions start failing bad, while n4sid still does well
With na=nb=4 (too high?) , the arx and plr also goes bad (large errs in both numerator and denom) Again n4sid good.
As can be seen the n4sid has typical coefficient errors in the e-8 range, while the others are up to e-1 Per Ove Husøy