Skip to content

Add extra information to nyquist plots #997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 30, 2024

Conversation

wueestry
Copy link
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

@murrayrm
Copy link
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)?

@coveralls
Copy link

coveralls commented Apr 26, 2024

Coverage Status

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

@wueestry
Copy link
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

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
Copy link
Member

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.

draws circles corresponding to the given magnitudes of sensitivity

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

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.

@wueestry wueestry force-pushed the nyquist_improvements branch from efc975f to 1f05e52 Compare May 24, 2024 12:40
@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
Copy link
Member

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

@murrayrm murrayrm merged commit 4acc78b into python-control:main Jun 30, 2024
23 checks passed
@murrayrm murrayrm added this to the 0.10.1 milestone Jun 30, 2024
@murrayrm murrayrm mentioned this pull request Jun 30, 2024
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants