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 — Add extra information to nyquist plots by wueestry · Pull Request #997 · python-control/python-control

Conversation

@wueestry

@wueestry wueestry commented Apr 24, 2024

Copy link
Copy Markdown
Contributor

Adds the possibility to add the unit circle (to get gain crossover frequency), Ms-circle (sensitivity) and Mt-circle (complementary sensitivity) to nyquist plots similar to the approach taken in JuliaControl

image

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@murrayrm

murrayrm commented Apr 26, 2024

Copy link
Copy Markdown
Member

It would be nice to have unit tests, even if they call the code to get coverage (better would be to check to make sure the lines are in the right place).

If the system is discrete time, are these still the right circles to draw?

Any interaction with axis scaling? Eg, if all of the action is very near the origin, might this reset the axes in a way that makes it harder to see the Nyquist plot? If so, is there a better way to do that (eg, fix the axes and let the circle disappear at the edges of the axes)?
All reactions

Sorry, something went wrong.

Uh oh!

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

coveralls commented Apr 26, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Coverage Status

coverage: 94.533% (+0.02%) from 94.518%
when pulling 10d2010 on wueestry:nyquist_improvements
into feeb56a on python-control:main.
All reactions

Sorry, something went wrong.

Uh oh!

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

wueestry commented Apr 30, 2024

Copy link
Copy Markdown
Contributor Author

It would be nice to have unit tests, even if they call the code to get coverage (better would be to check to make sure the lines are in the right place).

If the system is discrete time, are these still the right circles to draw?

Any interaction with axis scaling? Eg, if all of the action is very near the origin, might this reset the axes in a way that makes it harder to see the Nyquist plot? If so, is there a better way to do that (eg, fix the axes and let the circle disappear at the edges of the axes)?

This works as well for discrete systems. And tests have now been added.
Not sure yet if fixing the axis is the best idea, as it would result in issues when plotting a second system in the same plot afterwards
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
murrayrm
murrayrm reviewed May 11, 2024
View reviewed changes
Comment thread control/freqplot.py Outdated
If 'True', displays the unit circle, to read gain crossover frequency.

mt_circles : array_like, optional
draws circles corresponding to the given magnitudes of sensitivity

@murrayrm murrayrm May 11, 2024

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.

For consistency, start phrase with a capital letter and end in a period.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread control/freqplot.py Outdated
draws circles corresponding to the given magnitudes of sensitivity

ms_circles : array_like, optional
draws circles corresponding to the given magnitudes in complementary sensitivity

@murrayrm murrayrm May 11, 2024

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.

For consistency, start phrase with a capital letter and end in a period.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@wueestry
wueestry force-pushed the nyquist_improvements branch from efc975f to 1f05e52 Compare May 24, 2024 12:40
wueestry and others added 4 commits June 30, 2024 08:42
@wueestry @murrayrm
Add possibility to plot unit, ms and mt_circle for nyquist
f1af2e5
@wueestry @murrayrm
Adding sensitivity circle tests
c8cf6c3
@wueestry @murrayrm
Adding circle labels by default
2168b10
@murrayrm
rebase on main + allow circle styles to be set via defaults
10d2010
@murrayrm
murrayrm force-pushed the nyquist_improvements branch from ee68801 to 10d2010 Compare June 30, 2024 16:11
@murrayrm
murrayrm marked this pull request as ready for review June 30, 2024 16:11
@murrayrm

murrayrm commented Jun 30, 2024

Copy link
Copy Markdown
Member

I rebased this onto the current main branch and added the ability to set the line style for the circles using config.defaults.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@murrayrm
murrayrm merged commit 4acc78b into python-control:main Jun 30, 2024
@murrayrm murrayrm added this to the 0.10.1 milestone Jun 30, 2024
@murrayrm murrayrm mentioned this pull request Jun 30, 2024
Version 0.10.1 release plans #1020
Closed
11 tasks done
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.10.1

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

3 participants

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