use __call__ instead of evalfr in lti system classes#449
use __call__ instead of evalfr in lti system classes#449murrayrm merged 36 commits intopython-control:masterfrom
Conversation
…h evaluates at many frequencies at once
There was a problem hiding this comment.
There are a couple of failing unit tests to still iron out
May I point your interest towards #438, where some of the evalfr/_evalfr/freqresp etc. test have been treated by the iron already. Especially some continuous/discrete inconsistency.
|
@bnavigator any idea how soon #438 will be ready for prime time? I can plan to try to merge (and figure out how to do that) when you’re done with the PR. |
|
Hi, I think it is ready. But it depends on the yet unmerged PRs mentioned in the initial post. We should get those approved and merged first. I understand #438 is hard to review. The diff will get smaller with the underlying PRs though. |
sawyerbfuller
changed the title
|
A few comments: |
bnavigator
left a comment
bnavigator
left a comment
There was a problem hiding this comment.
Please keep the numpydoc style
Co-authored-by: Ben Greiner <[email protected]>
Co-authored-by: Ben Greiner <[email protected]>
doc fix Co-authored-by: Ben Greiner <[email protected]>
suggested doctoring fix for statesspace.slycot_horner Co-authored-by: Ben Greiner <[email protected]>
doctoring fixes to adhere to numpydoc convention Co-authored-by: Ben Greiner <[email protected]>
…and doing fallback
|
most recent commit moves the task of calling slycot and fall-back if it doesn't work into |
Co-authored-by: Ben Greiner <[email protected]>
Co-authored-by: Ben Greiner <[email protected]>
|
ok I think ive resolved doc comments with the last two commits Travis isnt happy about somethng though. |
|
Build 940 and 941 got canceled and 942 didn't start yet. We have to wait a little. And the day when we can ditch Travis for a different CI will be a happy day for me. (Working on Github Actions for Slycot right now in python-control/Slycot#140, and then I will tackle #446) |
|
Ok looks like Travis is happy now. I’m happy to have it merged but don’t understand the various options, happy to leave it to @murrayrm and @bnavigator discretion. |
5 participants
fixes problem reported in #434 and extends #179.
__call__as primary frequency response method to be used inTransferFunction,StateSpace, andFrequencyResponseDatasystems. (__call__is new forStateSpace, andFrequencyResponse)__call__has same interface for all three classes, and can now take one or an array ofs, unlike the old_evalfr, which could only take onessqueeze=Truethat automatically squeezes output ifsysis SISOFrequencyResponseData.__call__(s),smust be purely imaginary or an error is raised_evalfrthat was inconsistent with matlab modulematlab.evalfrwas removed in favor of__call__sys.evalfr(s)has been deprecated for 2.5 years and is now removed. use__call__instead, ormatlab.evalfr(sys, s)(following discussion in lti.evalfr and sys._evalfr have different behavior #434)sys.freqresp(omega)is now deprecated. Use new pythonic methodsys.frequency_resposeinstead, orfreqresp(sys, omega)from matlab module (following discussion in lti.evalfr and sys._evalfr have different behavior #434)horner(s)now does the same thing for bothTransferFunctionandStateSpacesystems: can evaluate at multiple values ofsFRD.evalbehavior was left as-is, except for an optional squeeze argument