Skip to content
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

Add argument to nyquistplot for critical point #688

Merged
merged 3 commits into from
Jun 1, 2022
Merged

Add argument to nyquistplot for critical point #688

merged 3 commits into from
Jun 1, 2022

Conversation

Zinoex
Copy link
Contributor

@Zinoex Zinoex commented Jun 1, 2022

Considering a Nyquist plot for the return difference F = 1 + L, we should mark the origin as the critical point instead of -1. Similarly, it allows plotting of the determinant of the return difference for a MIMO system, as we necessarily need to test encirclements of the origin. Default remains -1 as the critical point.

Considering a Nyquist plot for the return difference F = 1 + L, we should mark the origin as the critical point instead of -1. Similarly, it allows plotting of the determinant of the return difference for a MIMO system, as we necessarily need to test encirclements of the origin.
@albheim
Copy link
Member

albheim commented Jun 1, 2022

Is it common to have the system 1+L instead of L, or is it nicer to plot for some reason? Couldn't you just plot F-1 instead if you know your system is on that form?

Not saying it isn't a valid request, just want to understand the need since I have not encountered this myself and I don't immediately see any real benefit compared to just sending in F-1.

@baggepinnen
Copy link
Member

Maybe a good option would be to let the argument control the position of the point, so that

critical_point = -1

would be the default and

critical_point = 0

would mark the origin?

@codecov
Copy link

codecov bot commented Jun 1, 2022

Codecov Report

Merging #688 (ccb0756) into master (355d1cb) will not change coverage.
The diff coverage is 50.00%.

@@           Coverage Diff           @@
##           master     #688   +/-   ##
=======================================
  Coverage   87.45%   87.45%           
=======================================
  Files          34       34           
  Lines        3579     3579           
=======================================
  Hits         3130     3130           
  Misses        449      449           
Impacted Files Coverage Δ
src/plotting.jl 86.76% <50.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 355d1cb...ccb0756. Read the comment docs.

@Zinoex
Copy link
Contributor Author

Zinoex commented Jun 1, 2022

@albheim For a generalized Nyquist stability criterion, you plot det(I + L). Hence, you cannot modify the problem to encircle -1. Also, the Nyquist stability criterion is derived using the principle of the argument for encirclements around the origin for F = 1 + L, thus it may be desirable to plot F instead. I am by no means an expert in this area, but considering in particular the MIMO case, it is just a low-hanging fruit of 'nice-to-have'.

I have added the proposed change.

@baggepinnen
Copy link
Member

Cool, thanks! The argument should probably be reflected in the docstring as well, and then we're good to merge

@baggepinnen baggepinnen merged commit c67f0a0 into JuliaControl:master Jun 1, 2022
@baggepinnen
Copy link
Member

Thanks for your contribution, a new release will be out soon :)

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