Update DARE #173
Update DARE #173
Conversation
|
Looks good to me, @mfalt? |
src/matrix_comps.jl
Outdated
| B*inv(R)*B' | ||
| catch | ||
| error("R must be non-singular.") | ||
| if (minimum(eigvals(Q)) < 0) |
There was a problem hiding this comment.
What happens when eigvals are complex here?
|
Great! However two test cases where the new code breaks and the old code worked are The first test case is fixed by changing The second test case still breaks since somehow the new code doesn't handle scalar inputs due to that Two other test cases which would be good to include are |
Support complex/scalar A, B, Q, R.
|
Dear All, Thank you very much for your reviews! Also, I wrote a few test codes. |
| @@ -34,37 +34,35 @@ end | |||
| """`dare(A, B, Q, R)` | |||
|
|
|||
| Compute `X`, the solution to the discrete-time algebraic Riccati equation, | |||
| defined as A'XA - X - (A'XB)(B'XB + R)^-1(B'XA) + Q = 0, where A and R | |||
| are non-singular. | |||
| defined as A'XA - X - (A'XB)(B'XB + R)^-1(B'XA) + Q = 0, where Q>=0 and R>0 | |||
|
|
|||
| Algorithm taken from: | |||
There was a problem hiding this comment.
Does this need updating now?
| n = size(A, 1); | ||
|
|
||
| E = [ | ||
| Matrix{Float64}(I, n, n) B/R*B'; |
There was a problem hiding this comment.
| Matrix{Float64}(I, n, n) B/R*B'; | |
| I B/R*B'; |
shouldn't it be enough to write I?
There was a problem hiding this comment.
There was a bug in julia where this didn't work for 0 size of I. We could test it against 1.0.x to see if the proposed change works, but I don't see a problem in leaving it like this.
| ]; | ||
| F = [ | ||
| A zeros(size(A)); | ||
| -Q Matrix{Float64}(I, n, n) |
There was a problem hiding this comment.
| -Q Matrix{Float64}(I, n, n) | |
| -Q I |
same here?
|
Thanks for fixing this @HppyCtrlEngnrng |
Remove the assumption of invertibility of A matrix utilizing generalized Schur decomposition.