Skip to content

StateSpace.zero() now supports MIMO systems #205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 2, 2018
Merged

StateSpace.zero() now supports MIMO systems #205

merged 1 commit into from
Jul 2, 2018

Conversation

calcmogul
Copy link
Contributor

scipy.linalg.eigvals() is used to solve the generalized eigenvalue
problem. The zero locations used in the unit test were obtained via the
zero() function in MATLAB R2017b with two more digits of precision added
based on the results of StateSpace.zero().

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 77.707% when pulling 8607fb0 on calcmogul:mimo-zeros into 601b581 on python-control:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 77.707% when pulling 8607fb0 on calcmogul:mimo-zeros into 601b581 on python-control:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 77.707% when pulling 8607fb0 on calcmogul:mimo-zeros into 601b581 on python-control:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 77.707% when pulling 8607fb0 on calcmogul:mimo-zeros into 601b581 on python-control:master.

@coveralls
Copy link

coveralls commented May 28, 2018

Coverage Status

Coverage decreased (-13.1%) to 64.747% when pulling f66315d on calcmogul:mimo-zeros into 601b581 on python-control:master.

@rabraker
Copy link
Contributor

Calculating the zeros directly as a generalized eigenvalue problem only works if the system is square. The paper you linked talks about the non-square case on page 9.

Since square systems are common, I think this is a worthwhile pull request, but I do think the limitation should be noted in the doc string. I suppose a non-square system will cause scipy.linalg.eigvals() to throw its own error, but I think it would be better to catch it here, since we can provide a more enlightening error message.

@calcmogul
Copy link
Contributor Author

control.StateSpace.__init__() already requires that A is a square matrix and throws otherwise. However, documenting that zero()'s implementation details assume a square matrix seems reasonable. scipy.linalg.eigvals() throws ValueError('expected square matrix') if either argument is non-square, but the message is opaque.

@ilayn
Copy link

ilayn commented May 28, 2018

Alternatively (if available) ab08nd from slycot can be used to get the transmission zeros directly.

@calcmogul The squareness is about the system shape, 3 inputs and 2 outputs system cannot be used with the generalized eigenvalue method.

scipy.linalg.eigvals() is used to solve the generalized eigenvalue
problem. The zero locations used in the unit test were obtained via the
zero() function in MATLAB R2017b with two more digits of precision added
based on the results of StateSpace.zero().
@calcmogul
Copy link
Contributor Author

Oh, that makes a lot more sense. The latest patch uses AB08ND if available to produce square matrices scipy.linalg.eigvals() can use. I'd like the non-Slycot version to have feature parity though, so I'm going to keep working on that. The paper's suggestion of augmenting the non-square matrices with pseudo-random numbers isn't returning sensible eigenvalues for the test case (it always returns all 1's), but the input matrices to eigvals() look like what the paper specifies. I didn't like the idea of random numbers anyway.

I'm going to look into translating AB08ND's algorithm into Python instead. The logic and Fortran syntax hasn't been hard to follow so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants