Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
murrayrm merged 36 commits intopython-control:masterfromsawyerbfuller:call-method
Jan 8, 2021

Conversation

sawyerbfuller
Copy link
Contributor

@sawyerbfullersawyerbfuller commentedAug 14, 2020•
edited
Loading

fixes problem reported in#434 and extends#179.

  • move to__call__ as primary frequency response method to be used inTransferFunction,StateSpace, andFrequencyResponseData systems. (__call__ is new forStateSpace, andFrequencyResponse)
  • the new__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
  • it also adds a convenient keyword argumentsqueeze=True that automatically squeezes output ifsys is SISO
  • forFrequencyResponseData.__call__(s),s must be purely imaginary or an error is raised
  • private method_evalfr that was inconsistent with matlab modulematlab.evalfr was removed in favor of__call__
  • methodsys.evalfr(s) has been deprecated for 2.5 years and is now removed. use__call__ instead, ormatlab.evalfr(sys, s) (following discussion inlti.evalfr and sys._evalfr have different behavior #434)
  • methodsys.freqresp(omega) is now deprecated. Use new pythonic methodsys.frequency_respose instead, orfreqresp(sys, omega) from matlab module (following discussion inlti.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
  • de-duplication of code, cleanup, pythonization and vectorization of code where possible
  • FRD.eval behavior was left as-is, except for an optional squeeze argument

Copy link
Contributor

@bnavigatorbnavigator left a comment•
edited
Loading

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 theevalfr/_evalfr/freqresp etc. test have been treated by the iron already. Especially some continuous/discrete inconsistency.

@sawyerbfuller
Copy link
ContributorAuthor

@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.

@bnavigator
Copy link
Contributor

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 reacted with thumbs up emoji

@sawyerbfullersawyerbfuller changed the title[WIP] use __call__ instead of evalfr in lti system classesuse __call__ instead of evalfr in lti system classes [done]Aug 16, 2020
@coveralls
Copy link

coveralls commentedAug 16, 2020•
edited
Loading

Coverage Status

Coverage decreased (-0.07%) to 87.438% when pullinga631098 on sawyerbfuller:call-method into4103688 on python-control:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 84.032% when pulling2c4ac62 on sawyerbfuller:call-method intod3142ff on python-control:master.

@sawyerbfuller
Copy link
ContributorAuthor

sawyerbfuller commentedAug 16, 2020•
edited
Loading

A few comments:sys.horner was modified slightly so that it only takes care of the low-level algorithm, while__call__ is intended to perform user friendly tasks like input formatting and squeezing the output as necessary.

Copy link
Contributor

@bnavigatorbnavigator left a 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

Copy link
Contributor

@bnavigatorbnavigator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@bnavigatorbnavigator added this to the0.9.0 milestoneAug 16, 2020
@bnavigatorbnavigator linked an issueAug 16, 2020 that may beclosed by this pull request
sawyerbfullerand others added10 commitsAugust 17, 2020 10:49
Co-authored-by: Ben Greiner <code@bnavigator.de>
Co-authored-by: Ben Greiner <code@bnavigator.de>
doc fixCo-authored-by: Ben Greiner <code@bnavigator.de>
suggested doctoring fix for statesspace.slycot_hornerCo-authored-by: Ben Greiner <code@bnavigator.de>
doctoring fixes to adhere to numpydoc conventionCo-authored-by: Ben Greiner <code@bnavigator.de>
@sawyerbfuller
Copy link
ContributorAuthor

most recent commit moves the task of calling slycot and fall-back if it doesn't work intosys.horner

Co-authored-by: Ben Greiner <code@bnavigator.de>
Co-authored-by: Ben Greiner <code@bnavigator.de>
@sawyerbfuller
Copy link
ContributorAuthor

sawyerbfuller commentedJan 7, 2021•
edited
Loading

ok I think ive resolved doc comments with the last two commits

Travis isnt happy about somethng though.

@bnavigator
Copy link
Contributor

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 inpython-control/Slycot#140, and then I will tackle#446)

sawyerbfuller reacted with thumbs up emoji

@sawyerbfuller
Copy link
ContributorAuthor

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.

@murrayrmmurrayrm merged commit0eb4396 intopython-control:masterJan 8, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@bnavigatorbnavigatorbnavigator approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.9.0
Development

Successfully merging this pull request may close these issues.

lti.evalfr and sys._evalfr have different behavior
5 participants
@sawyerbfuller@bnavigator@coveralls@roryyorke@murrayrm
[8]ページ先頭
©2009-2025 Movatter.jp