GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix gain handling in rlocus and sisotool by sawyerbfuller · Pull Request #809 · python-control/python-control

Conversation

@sawyerbfuller

@sawyerbfuller sawyerbfuller commented Dec 7, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

current problem is that if you want to pass an initial gain to sisotool to plot as starting point gain in the various plots, an error is raised if it is not an array. so you have to do sisotool(sys, kvect=(1.1, )) which is awkward. With this fix, you can do sisotool(sys, kvect=1.1) update: changed function call to look like sisotool(sys, initial_gain=1.1)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sawyerbfuller
allow sisotool to receive kvect as a singleton rather than always an …
3854050
…array
@coveralls

coveralls commented Dec 7, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 94.785% when pulling 15543ff on sawyerbfuller:kvect into 9d65bf8 on python-control:main.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sawyerbfuller added 5 commits December 9, 2022 10:03
@sawyerbfuller
improved docstring for kvect in sisotool
feb9010
@sawyerbfuller
docstring improvements, pep8 cleanup, more descriptive names for inte…
e808adb
…rnal variables
@sawyerbfuller
introduce new keyword for sisotool: initial_gain, which denotes gain …
4107950
…that the plots start with. Deprecate kvect kwarg with warning. rlocus now doesn't recomupte kvect if it is passed
@sawyerbfuller
test to make sure kvect gains are respected even if plotting
eadd496
@sawyerbfuller
rename mymat to root_array for clarity
15543ff
@sawyerbfuller

sawyerbfuller commented Dec 9, 2022

Copy link
Copy Markdown
Contributor Author

Added some more fixes to the kvect kwarg in rlocus and sisotool. kvect was intended to let the user specify their own array of gains, but it wasn't being used that way. Two primary changes aim to make it do what I believe was its intended purpose:

  1. I deprecated kvect in sisotool, because you probably don't want to specify your own set of gains to plot in the rlocus plot in sisotool anyway. You probably want to use the ones that are computed automatically. Especially because sisotool/rlocus allows you to zoom in, and it re-computes gain points for the new zoomed root locus plot, to get a more precise click. Instead, there is now a new keyword, initial_gain that allows the user to specify the starting value of the closed-loop gain to plot in its four plots. kvect can be still given, and it reproduces the old behavior of sisotool using kvect[0] as the initial gain, and ignoring everything else in kvect. And there is now a deprecation warning.

  2. For rlocus, you might actually want to specify your own kvect if for example you only want to see where the roots go for a certain set of gains. But this wasn't what was happening: the rlocus plot ignores the kvect argument altogether and essentially always uses its own pre-computed gains. This is because any time the axis range is changed, kvect is automatically recomputed, which is basically every time the plot is created. I fixed it so that the new behavior is that if kvect is supplied to rlocus, it is no longer recomputed automatically if the plot is zoomed.

  3. Other improvements:

  • docstring fixes
  • pep8 cleanups
  • change mymat to root_array in rlocus to better describe its function
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sawyerbfuller sawyerbfuller changed the title allow sisotool to receive kvect as a singleton or an array fix gain handling in rlocus and sisotool Dec 9, 2022
murrayrm
murrayrm reviewed Dec 14, 2022
View reviewed changes

@murrayrm murrayrm left a comment

Copy link
Copy Markdown
Member

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.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@murrayrm murrayrm linked an issue Dec 14, 2022 that may be closed by this pull request
rlocus with kvect argument doesn't compute good limits #787
Closed
@murrayrm
murrayrm merged commit b1d55a1 into python-control:main Dec 16, 2022
@murrayrm murrayrm added this to the 0.9.3 milestone Dec 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@murrayrm murrayrm murrayrm left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.9.3

Development

Successfully merging this pull request may close these issues.

rlocus with kvect argument doesn't compute good limits

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@sawyerbfuller @coveralls @murrayrm
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.