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 — Update Nyquist rescaling + other improvements by murrayrm · Pull Request #1155 · python-control/python-control

Conversation

@murrayrm

@murrayrm murrayrm commented Jun 21, 2025

Copy link
Copy Markdown
Member

This PR addresses issue #1143 by changing the way that rescaling is handled in nyquist_plot. The prior code saturated the Nyquist curve at max_curve_magnitude, which would leave to confusing plots when the Nyquist curve had interesting features at large magnitude, since all of that detail was "collapsed" onto the limiting circle. In the new code, a 1-1 rescaling function is used that maps large magnitude features into a continuous range of magnitudes starting at a "blend point" and going out to max_curve_magnitude. The result is that the features of the Nyquist curve can be more easily visualized. The fraction of the Nyquist curve that is "blended" is set with a new parameter blend_fraction, that defaults to 0.20 (so the last 20% of the Nyquist curve prior to max_curve_magnitude is rescaled, with everything from the blend point to $\infty$ mapping from the blend point to max_curve_magnitude.

Using the example that @JonHowMIT raised in issue #1143, we go from this:

Figure_1_orig

to this:

Figure_1_new

The original behavior can be retained by setting blend_fraction=0.

A few other changes that are also part of this PR:

  • Updated max_curve_magnitude from a default of 20 to 15 (this allows the -1 point to be see better).
  • Updated the default number of arrows from 2 to 3 (looked better in the gallery of plots available in control/tests/nyquist_test.
  • Modified control/tests/nyquist_test to generate a gallery of plots when run as a script (via python or ipython).
  • Changed the output of nyquist_plot to match its documentation. In particular, cplt.lines was returning a 1D array instead of a 2D array (as done in all other _plot functions).
  • Updated unit tests and examples.

Sorry, something went wrong.

Uh oh!

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

All reactions
murrayrm added 5 commits June 21, 2025 01:34
@murrayrm
Update Nyquist rescaling + other improvements
ca6f3da
@murrayrm
ruff check fixes
5aca44a
@murrayrm
updated documentation (user guide figures + docstrings)
f37656b
@murrayrm
update nyquist_plot lines output to match documentation
51fa137
@murrayrm
update custom Nyquist plot in documentation
13eda42
@coveralls

coveralls commented Jun 21, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Coverage Status

coverage: 94.747% (-0.01%) from 94.759%
when pulling cf3daa4 on murrayrm:fix_nyquist_rescaling-24Mar2025
into 34c6d59 on python-control:main.

All reactions

Sorry, something went wrong.

Uh oh!

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

@slivingston
slivingston self-requested a review June 22, 2025 17:59
slivingston
slivingston reviewed Jun 24, 2025
View reviewed changes

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

I made several code quality comments. In terms of clarity of plots, I think this new rescaling behavior is good and ready to merge.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread control/tests/nyquist_test.py Outdated
from datetime import date

# Create the file to store figures
git_info = subprocess.check_output(['git', 'describe'], text=True).strip()

@slivingston slivingston Jun 24, 2025

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.

Suggested change
git_info = subprocess.check_output(['git', 'describe'], text=True).strip()
try:
git_info = subprocess.check_output(['git', 'describe'], text=True).strip()
except subprocess.CalledProcessError:
git_info = 'UNKNOWN-REPO-INFO'

This will fail if the tests are run from outside the Git repository directory, which is possible from anywhere using

python -m control.tests.nyquist_test

To handle this, I recommend to either default to something like "UNKNOWN" or to warn the user ("unable to get version info from git") and not include {git_info} in the file name.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread control/freqplot.py Outdated
Comment on lines +1902 to +1904
if blend_fraction < 0 or blend_fraction > 1:
raise ValueError("blend_fraction must be between 0 and 1")
blend_curve_start = (1 - blend_fraction) * max_curve_magnitude

@slivingston slivingston Jun 24, 2025

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.

Neither checking feasibility of blend_fraction nor declaring blend_curve_start depends on the loop variables (idx and response), so these can be moved to before the start of this for-loop.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread control/freqplot.py Outdated
abs_subset = np.abs(subset)
unit_vectors = subset / abs_subset # Preserve phase/direction

if blend_curve_start is None or \

@slivingston slivingston Jun 24, 2025

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.

blend_curve_start is never None, so the first clause (blend_curve_start is None) can be deleted.

Sorry, something went wrong.

Uh oh!

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

All reactions
@murrayrm
address @slivingston review comments
cf3daa4
@murrayrm murrayrm linked an issue Jun 25, 2025 that may be closed by this pull request
Nyquist plot rescaling is confusing #1143
Closed
@murrayrm

murrayrm commented Jun 25, 2025

Copy link
Copy Markdown
Member Author

Thanks for the careful review @slivingston. Incorporated your comments and will merge as soon as unit tests pass.

All reactions

Sorry, something went wrong.

Uh oh!

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

@murrayrm
murrayrm merged commit bb47d59 into python-control:main Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

No milestone

Development

Successfully merging this pull request may close these issues.

Nyquist plot rescaling is confusing

Uh oh!

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

3 participants

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