- Notifications
You must be signed in to change notification settings - Fork441
fixed StateSpace for complex inputs#468
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| 'statesp.use_numpy_matrix':True, | ||
| 'statesp.use_numpy_matrix':False, |
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.
Please don't change the default setting here. We have a road map in mind when this should happen and it should be independent of the support for complex types.
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.
That said, actually#438 takescomplex inputs into account already!
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.
Ok, I will change this back
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.
That said, actually#438 takescomplex inputs into account already!
Oh, I see, should I close my PR?
Oh, I wasn`t aware of those discussions. |
Travis is again not running, but without testing, I think your change will create a lot of casting warnings for cases, when users use control and expect real valued results. Given that#438 takes care of the correct type depending on the input, it should also resolve your use-case. That PR has merge-conflicts with current master right now, but are you able to check it out and test it anyway? |
I closed it because the tests fail and the issue is presumably resolved in another PR. Please feel free to open a new one if you think there is a better way than the approach in#438 or that your example script is a useful enhancement of the existing examples. But I honestly fail to see the point of the example beyond the fact that it demonstrates that python-control can't deal with complex state-space matrices yet. |
The problem was that when trying to simulate a system with complex values in the state space matrices it was by default casting to float, which generated wrong results.
I had to create a pull request to scipy as well as the dlsim function has a similar issue:
scipy/scipy#13075
I also created an example script where this problem showed up and was fixed.