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 blank bode plot in rootlocus_pid_designer by sawyerbfuller · Pull Request #883 · python-control/python-control

Conversation

@sawyerbfuller

@sawyerbfuller sawyerbfuller commented Apr 14, 2023

Copy link
Copy Markdown
Contributor

Starts with a small gain rather than zero to insure that the frequency response plots appear. Addresses #686

Sorry, something went wrong.

Uh oh!

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

All reactions
@sawyerbfuller
fix blank bode plot in rootlocus_pid_designer
a0877c5
@sawyerbfuller sawyerbfuller linked an issue Apr 14, 2023 that may be closed by this pull request
rootlocus_pid_designer plot looks strange + needs documentation #686
Closed
@coveralls

coveralls commented Apr 14, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Coverage Status

Coverage: 94.649% (+0.1%) from 94.535% when pulling fabcb37 on sawyerbfuller:rlpd_blank into 1e1c8eb on python-control:main.

All reactions

Sorry, something went wrong.

Uh oh!

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

@murrayrm

murrayrm commented Apr 15, 2023

Copy link
Copy Markdown
Member

I tried running this on the example I pointed out in #686 and when I click on the root locus, nothing changes. I then tried the same system in sisotool and that also didn't work. I'm assuming this works for others? Any clues about what I am doing wrong?

Also, why is 0.001 the right initial_gain to use? We have a default value for this (sisotool.initial_gain) that is set to 1. Shouldn't we use that instead?

All reactions

Sorry, something went wrong.

Uh oh!

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

@murrayrm
TRV: fix docstring typo in rootlocus_pid_designer
c3a013b
@murrayrm

murrayrm commented Apr 15, 2023

Copy link
Copy Markdown
Member

Also, in looking at the code a bit, I am confused about the fact that the initial gain seems to be set using the Kp0, Ki0, and Kd0 parameters, but then it is reset to 0.001 for the gain that you choose to vary. Should you be able to override this through one of those parameters?

All reactions

Sorry, something went wrong.

Uh oh!

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

@sawyerbfuller

sawyerbfuller commented Apr 16, 2023

Copy link
Copy Markdown
Contributor Author

Regarding default gain: for rootlocus-pid-designer, in principle you are already starting with baseline nonzero kd0, kp0, and ki0, and the step responses show result of that. You are interested in any addition delta-k to one of those, so it makes sense to start that one with a small value. But a zero value erases Bose plot. Np.eps is another option but it would make a very low gain bode plot so I picked a kind of arbitrary small nonzero value.

All reactions

Sorry, something went wrong.

Uh oh!

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

@sawyerbfuller

sawyerbfuller commented Apr 16, 2023

Copy link
Copy Markdown
Contributor Author

Re non-interactive plot: doesn’t work in Jupiter, have to use magic command %matplotlib qt. Assuming you tried that, can you zoom? The way it works you have to click near one of its pre calculated points. It computes more if you zoom in.

All reactions

Sorry, something went wrong.

Uh oh!

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

sawyerbfuller added 4 commits April 16, 2023 13:15
@sawyerbfuller
added noteto docstring for how to use when no interative plots
5d7a8d3
@sawyerbfuller
merge
7f4e2ed
@sawyerbfuller
enable deltaK in sisotool for non-interactive use, docstring improvem…
0fb9d94
…ents
@sawyerbfuller
added new kwarg to tests, un-skip tests that were previously giving w…
74d96f0
…arnings and problems.
@sawyerbfuller

sawyerbfuller commented Apr 20, 2023

Copy link
Copy Markdown
Contributor Author

@murrayrm On further thought based on your questions, I realized that a deltaK parameter would be useful needed for non-interactive use, so that the bode plot was usable. Added it in.

All reactions

Sorry, something went wrong.

Uh oh!

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

@murrayrm
add %matplotlib note + tweak docstrings
fabcb37
@murrayrm

murrayrm commented Apr 23, 2023

Copy link
Copy Markdown
Member

Using %matplotlib fixed the issues I was having, so I added a bit of documentation.

sawyerbfuller reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

@murrayrm
murrayrm merged commit c06a205 into python-control:main Apr 23, 2023
@murrayrm murrayrm added this to the 0.9.4 milestone Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

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

Development

Successfully merging this pull request may close these issues.

rootlocus_pid_designer plot looks strange + needs documentation

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.