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 — Bug fix and improvements to Nyquist plots by murrayrm · Pull Request #722 · python-control/python-control

Conversation

@murrayrm

@murrayrm murrayrm commented Apr 15, 2022

Copy link
Copy Markdown
Member

This PR fixes a bug in the the way that the nyquist_plot command was implemented and introduces a bunch of enhancements, including some of the ideas in PR #671.

Bug fix: because of the new default parameter settings introduced in #534, it was possible for the indentation around an open-loop pole that was near the imaginary axis to exclude a region in the right half-plane that contained a closed-loop pole (so, for example, if you had a stable open-loop pole at -0.05 and an unstable closed-loop pole at 0.05, the default indentation of 0.1 would cause you to miss the encirclement). This has now been fixed by (1) generating a warning when the indented contour bypasses a closed loop pole and (2) implementing new functionality that allows the default indentation to be reduced (to 1e-4) while still keeping the magnitude of the Nyquist plot within reasonable limits.

Enhancements: in fixing the bug above, it was necessary to find a better balance between the size of the indentation and the magnitude of the Nyquist curve. This is also the goal of PR #671. The following changes have been done to implement this:

  • There is now a max_curve_magnitude function that limits the size of the plotted Nyquist curve. The default value for this parameter is 20, which is small enough that that -1 point is still visible in the plot. All points on the curve that are greater than the maximum magnitude specified are scaled so that the angle of the curve is preserved, but the magnitude is limited. The value for this parameter can be passed as a keyword argument or set using ct.defaults.
  • Portions of the Nyquist curve that have been scaled as well as portions of the curve corresponding to segments of the Nyquist contour that have been indented are now plotted in a different line style, so that it is more clear that these portions of the curve are not quantitatively accurate. The line styles can be set using the primary_linestyle and mirror_linestyle keywords and default to ['-', '-.'] and ['--', ':'].
  • The logic for inserting indentations has been improved and the indentations are now done based on the location of near imaginary poles. This results in smoother curves near zero frequency and more accurate curves around near-imaginary poles.
  • A new curve_offset keyword can be used so that overlapping sections of the Nyquist curve (typically at the limit values set by max_curve_magnitude are now separated out by a small amount (by adjusting the primary curve outward and the mirror curve inward by a small amount that is smooth at the end of the scaled sections).
  • A marker is placed at the zero frequency point on the Nyquist curve, to help add in determining where the curve "starts". The type of marker is controlled by the start_marker keyword and the size of the marker is controlled with the start_marker_size keyword (default = a filled circle of size 4).
  • Several warnings have been added for situations where the Nyquist plot may not be an accurate representation:
    • If the number of net encirclements is not an integer (i.e., the Nyquist curve is not closed). This warning can be turned off using warn_encirclements=False.
    • If an indention occurs around an open-loop pole that might cause a closed-loop pole to occur outside the (indented) Nyquist contour.
    • If there are open-loop poles on the imaginary access and indentation is turned off (which can result in an incorrect Nyquist curve).
    • If the Nyquist criterion is not met (number of encirclements does not match the stability of the closed-loop system, taking into account closed-loop poles).
  • The original behavior (parameter settings) of v0.9.1 can be obtained using use_legacy_defaults('0.9.1').

The new features can be seen in the following example

sys = ct.tf([1], [1, 3, 2]) * ct.tf([1], [1, 0, 1])
count = ct.nyquist_plot(sys)

which produces the following plot:
Figure_18
Features to note:

  • Use of dash-dot and dotted line styles to mark sections of the curve that have been scaled.
  • Use of a marker at zero frequency
  • Offset between the primary curve and mirror curve at large magnitudes (so that they don't completely overlap)

Comparisons between old and new Nyquist plots:

Old New Comments
Figure_2 Figure_2 Better curves at zero frequency (with marker)
Figure_4 Figure_4 Better default scaling for curves with large magnitude (and marking of scaled portion of curve)
Figure_12 Figure_12 Better marking of scaled portion of the curve
Figure_11 Figure_11 No change to "regular" Nyquist plots (except for the marker at zero frequency).

Sorry, something went wrong.

Uh oh!

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

All reactions
@coveralls

coveralls commented Apr 15, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.1%) to 94.306% when pulling 7407265 on murrayrm:nyquist_bug_09Apr2022 into 2102181 on python-control:master.

All reactions

Sorry, something went wrong.

Uh oh!

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

murrayrm added 10 commits April 15, 2022 17:52
@murrayrm
add warning for nyquist_plot() when indent_radius is too large
18e997c
@murrayrm
add warning for nyquist_plot() when Nyquist criterion isn't met
1e12a51
@murrayrm
add code to limit magnitude of response and enhance contour near poles
6da8706
* use plot_curve_magnitude to scale response at large magnitudes
* add new line styles of scaled points on the curve
* add points to contour near imaginary poles to avoid missing encirclements
* add offsets to primary/mirror curve at large amplitude to avoid overlap
* (still needs some unit tests, documention updates, and comparisons)
@murrayrm
smooth curve offsets to avoid discontinuities
3d3a559
@murrayrm
tweak max_curve_magnitude + add start point
aef3b06
@murrayrm
add warning if encirclements is not near an integer
c25c0cb
@murrayrm
add legacy settings for nyquist
67c1f3f
@murrayrm
increase coverage; PEP8 cleanup
9a5759f
@murrayrm
turn off encirclement warnings for describing function plot
5dd6a70
@murrayrm
rebase cleanup
5d9c7f0
@murrayrm
murrayrm force-pushed the nyquist_bug_09Apr2022 branch from f3dfe04 to 5d9c7f0 Compare April 16, 2022 00:59
@murrayrm murrayrm mentioned this pull request Apr 16, 2022
Discontinuous Nyquist Plot #691
Closed
roryyorke
roryyorke reviewed Apr 16, 2022
View reviewed changes

@roryyorke roryyorke left a comment

Copy link
Copy Markdown
Contributor

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.

This looks great. I didn't understand the detail of the plotting code, but it looks clear enough; just complicated!

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread control/freqplot.py Resolved

Uh oh!

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

Comment thread control/freqplot.py Resolved

Uh oh!

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

Comment thread control/freqplot.py Resolved

Uh oh!

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

@murrayrm
Fixed docstrings per roryyorke plus discrete-time corrections
7407265
* changed return type for poles() and zeros() to complex
* updated (missing) discrete-time tests for stability warnings
* changed processing of near imaginary poles to pure imaginary poles
sawyerbfuller
sawyerbfuller reviewed Apr 16, 2022
View reviewed changes
Comment thread control/freqplot.py
- (s - p).real

# Figure out which way to offset the contour point
if p.real < 0 or (p.real == 0 and

@sawyerbfuller sawyerbfuller Apr 16, 2022

Copy link
Copy Markdown
Contributor

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.

I think I was the one who changed this code from equality to zero to is close to zero for the reason that sometimes when working with discrete systems you start with a continuous time system, with poles on imag axis, and then discretize it. When these poles are mapped back to the s plane in this code, their real part doesn’t end up to be perfectly zero anymore.

I Probably should have written a unit test!

Sorry, something went wrong.

Uh oh!

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

All reactions

@murrayrm murrayrm Apr 17, 2022

Copy link
Copy Markdown
Member Author

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.

The current code should OK because the Nyquist criterion is "robust" with respect to whether you indent to the right or the left. You just have to be consistent in how you count open and closed loop poles.

Here's an example (from tests/nyquist_test.py):

sys_c = ct.tf([1], [1, 3, 2]) * ct.tf([1], [1, 0, 1])
sys = ct.c2d(sys_c, 0.01)
np.abs(sys.poles()) - 1
array([ 9.58553237e-11,  9.58553237e-11, -9.95016656e-03, -1.98013266e-02])

The closed loop system has poles at

sys.feedback().poles()
array([1.00116706+0.01054462j, 1.00116706-0.01054462j,
       0.98390719+0.00189796j, 0.98390719-0.00189796j])

so it is unstable (as is the continuous time system, which has a pair of poles with real part 0.122).

If you run the current code, it will map the z-plane poles to

np.log(sys.poles()) / sys.dt
array([ 9.58553380e-09+1.j,  9.58553380e-09-1.j, -1.00000003e+00+0.j,
       -1.99999999e+00+0.j])

Note that what used to be a purely imaginary pole (at +/- 1j) is now slightly unstable (as you described). But the Nyquist criterion is still OK: we indent to the left (since they are unstable) and we end up with zero net encirclements:

count = ct.nyquist_plot(sys)

Figure_1

Since Z = N + P = 0 + 2 = 2, we conclude that the closed loop system has two poles in the right half-plane (correct!).

If it had turned out that the poles had mapped to exactly on the unit circle or to just inside the unit circle, then we would get a plot that looks like the continuous time plot for this example:
Figure_2
Note that in this case there are 2 encirclements and we get Z = N + P = 2 + 0 = 2, which is the same as before.

Sorry, something went wrong.

Uh oh!

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

All reactions
@murrayrm
murrayrm merged commit 46b3c53 into python-control:master Apr 25, 2022
@murrayrm
murrayrm deleted the nyquist_bug_09Apr2022 branch April 26, 2022 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sawyerbfuller sawyerbfuller sawyerbfuller left review comments
+1 more reviewer
@roryyorke roryyorke roryyorke left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

4 participants

@murrayrm @coveralls @roryyorke @sawyerbfuller
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.