- Notifications
You must be signed in to change notification settings - Fork441
Add unit test illustrating issue #935 + add method keyword for tf2ss#937
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Add unit test illustrating issue #935 + add method keyword for tf2ss#937
Uh oh!
There was an error while loading.Please reload this page.
Conversation
coveralls commentedOct 22, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@@ -2250,6 +2274,8 @@ def _convert_to_statespace(sys, use_prefix_suffix=False): | |||
A, B, C, D = \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Suggested change for the two lines above:
if not issiso(sys): raise ControlMIMONotImplemented("MIMO system conversion not supported without Slycot")
@@ -2250,6 +2274,8 @@ def _convert_to_statespace(sys, use_prefix_suffix=False): | |||
A, B, C, D = \ | |||
sp.signal.tf2ss(squeeze(sys.num), squeeze(sys.den)) | |||
newsys = StateSpace(A, B, C, D, sys.dt) | |||
else: | |||
raise ValueError(f"unknown {method=}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
First instantiation of self-documenting F-strings in python-control? : )
This PR puts in some unit tests associated with issue#935, which looks like it is a problem in slycot where the tf2ss functionality (in
td04ad
) doesn't work properly. As a partial workaround, amethod
keyword is added totf2ss
(andss
) that allowsmethod='scipy'
to be used for SISO systems. Docstrings updated for the new keyword and a note on theslycot
issue.A bug report has been created in the
slycot
repository (#222 and the unit tests will fail when the bug is fixed (so we can go back and update the unit tests and documentation).