Repository navigation
Return type of eigenvalues from lqr and lqe not consistent #439
Description
Activity
- I am not sure what the convention is here but I saw elsewhere that there were functions that returned a matrix or an array depending on that setting, using _ssmatrix. So my take is that Lqr should probably do the same. SOn Tue, Jul 28, 2020 at 5:49 PM Ben Greiner ***@***.***> wrote: This came up while pytestifying statefbk_test.py (#438 <#438>) lqr() will always return E as (nstates, ) array https://github.com/python-control/python-control/blob/d3142ff24c7af5e2c0843fd16d0e1be82b322bb9/control/statefbk.py#L444-L446 lqe() returns _ssmatrix(E) which is a (nstates, ) array or (1, nstates) matrix depending on config.defaults.use_numpy_matrix() https://github.com/python-control/python-control/blob/d3142ff24c7af5e2c0843fd16d0e1be82b322bb9/control/statefbk.py#L290 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#439>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AN74SSOF6TGZCDRQP5I2WBDR55WYBANCNFSM4PLCB64A> .-- Sawyer Fuller Assistant Professor of Mechanical Engineering University of Washington http://faculty.washington.edu/~minster/ (Typed with my thumbs)
I agree :)
See also #418
I'd like to clean this up before 0.8.4 release, but there are a few issues that could break users' code:
lqrreturns_ssmatrix(K), _ssmatrix(S), E, for which the first return value is the gain matrix (2D array or matrix, depending on config settings), the second return value is the solution to the Riccati equation (2D array or matrix), and the third value is a 1D array (list of closed loop eigenvalues).lqereturns_ssmatrix(K), _ssmatrix(S), _ssmatrix(E), which forces the third argument to be a 2D array.careanddarereturn_ssmatrix(X), w[:n], _ssmatrix(G), for which the first return value is the solution to the Riccati equation (2D array or matrix), the second return value is a list of eigenvalues (1D array), and the third argument is the gain matrix (2D array or matrix).
I think we should make the following change in 0.8.4:
- Change return values for
lqeto matchlqr
This will require an updated unit test (as noted above) and could break some users' code. But the fix to user code is simple (convert to 2D array) and so I think it is OK for 0.8.4.
I think we might want to consider whether to make changes to
careanddarein 0.9.0:-
We can leave things the same with the argument that
careanddareare returning solutions to the Riccati equation as their primary usage => that solution should be first. Thelqrandlqefunctions callcareordare, but are returning controllers as their primary usage => OK for the order to be different. -
We can change the order of the return values for
careanddareto matchlqrandlqe.
My current feeling is that we leave
careanddarealone. Thoughts?Reacted by Ben Greiner- Agree with richards’ take that ok for care and dare to have different return order. Off the top of my head I think matlab might do the same. Re lqe return argument - I think I made a mistake when I originally wrote it to naively turn E into an ssmatrix. Agree it should not have been. SOn Thu, Dec 24, 2020 at 9:37 AM Richard Murray ***@***.***> wrote: I'd like to clean this up before 0.8.4 release, but there are a few issues that could break users' code: - lqr returns _ssmatrix(K), _ssmatrix(S), E, for which the first return value is the gain matrix (2D array or matrix, depending on config settings), the second return value is the solution to the Riccati equation (2D array or matrix), and the third value is a 1D array (list of closed loop eigenvalues). - lqe returns _ssmatrix(K), _ssmatrix(S), _ssmatrix(E), which forces the third argument to be a 2D array. - care and dare return _ssmatrix(X), w[:n], _ssmatrix(G), for which the first return value is the solution to the Riccati equation (2D array or matrix), the second return value is a list of eigenvalues (1D array), and the third argument is the gain matrix (2D array or matrix). I think we should make the following change in 0.8.4: - Change return values for lqe to match lqr This will require an updated unit test (as noted above) and could break some users' code. But the fix to user code is simple (convert to 2D array) and so I think it is OK for 0.8.4. I think we might want to consider whether to make changes to care and dare in 0.9.0: - We can leave things the same with the argument that care and dare are returning solutions to the Riccati equation as their primary usage => that solution should be first. The lqr and lqe functions call care or dare, but are returning controllers as their primary usage => OK for the order to be different. - We can change the order of the return values for care and dare to match lqr and lqe. My current feeling is that we leave care and dare alone. Thoughts? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#439 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AN74SSLASGB633QJ46ETPLTSWN3ZFANCNFSM4PLCB64A> .-- Sawyer Fuller Assistant Professor of Mechanical Engineering Adjunct, Paul G Allen School of Computer Science University of Washington http://faculty.washington.edu/~minster/ (Typed with my thumbs)
This came up while pytestifying statefbk_test.py (#438)
lqr()will always returnEas(nstates, ) arraypython-control/control/statefbk.py
Lines 444 to 446 in d3142ff
lqe()returns_ssmatrix(E)which is a(nstates, ) arrayor(1, nstates) matrixdepending onconfig.defaults.use_numpy_matrix()python-control/control/statefbk.py
Line 290 in d3142ff