-
Notifications
You must be signed in to change notification settings - Fork 438
use __call__ instead of evalfr in lti system classes #449
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
Conversation
…h evaluates at many frequencies at once
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.
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. |
A few comments: |
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 keep the numpydoc style
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.
more numpydoc
Co-authored-by: Ben Greiner <code@bnavigator.de>
Co-authored-by: Ben Greiner <code@bnavigator.de>
doc fix Co-authored-by: Ben Greiner <code@bnavigator.de>
suggested doctoring fix for statesspace.slycot_horner Co-authored-by: Ben Greiner <code@bnavigator.de>
doctoring fixes to adhere to numpydoc convention Co-authored-by: Ben Greiner <code@bnavigator.de>
…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 <code@bnavigator.de>
Co-authored-by: Ben Greiner <code@bnavigator.de>
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. |
fixes problem reported in #434 and extends #179.
__call__
as primary frequency response method to be used inTransferFunction
,StateSpace
, andFrequencyResponseData
systems. (__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 ones
squeeze=True
that automatically squeezes output ifsys
is SISOFrequencyResponseData.__call__(s)
,s
must be purely imaginary or an error is raised_evalfr
that was inconsistent with matlab modulematlab.evalfr
was 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_respose
instead, 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 bothTransferFunction
andStateSpace
systems: can evaluate at multiple values ofs
FRD.eval
behavior was left as-is, except for an optional squeeze argument