Skip to content

Discrete omega-damping plot and tweaks #193

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.

Sign up for GitHub

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
murrayrm merged 5 commits into from
Dec 28, 2018
Merged

Discrete omega-damping plot and tweaks #193

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dc8410c
Added discrete grid to pzmap function.
Feb 19, 2018
4117489
Revert "Added discrete grid to pzmap function."
Feb 19, 2018
f8c497e
Added discrete grid to pzmap function.
Feb 19, 2018
3d0bf03
Tweaked axis lines and zero/pole markers.
Feb 19, 2018
89dc937
Refactored sgrid based on matplotlib projections and moved sgrid and …
Mar 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 41 additions & 6 deletions control/pzmap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,42 @@
#
# $Id:pzmap.py 819 2009-05-29 21:28:07Z murray $

from numpy import real, imag
from .lti import LTI
from numpy import real, imag, linspace, pi, exp, cos, sin, sqrt

@murrayrm murrayrm Feb 23, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than using numpy.pi, please use math.pi to avoid problems with sphinx. See commit aa6e0df

@Sup3rGeo Sup3rGeo Feb 25, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

from .lti import LTI, isdtime, isctime

__all__ = ['pzmap']

def zgrid(plt):

@murrayrm murrayrm Feb 23, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zgrid function needs a docstring + some comments in the code to describe what is going on.

@Sup3rGeo Sup3rGeo Feb 25, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! Do you have any example I could be based on?

@murrayrm murrayrm Feb 25, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a look at nichols.py and, in particular, the nichols_grid() function.

for zeta in linspace(0, 0.9,10):

@murrayrm murrayrm Feb 23, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The range for zeta should probably be something with a default set of parameters that can be reset by the user.

@Sup3rGeo Sup3rGeo Feb 25, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

making zetas and wns arguments so user can override default behaviour.

factor = zeta/sqrt(1-zeta**2)
x = linspace(0, sqrt(1-zeta**2),200)
ang = pi*x
mag = exp(-pi*factor*x)
xret = mag*cos(ang)
yret = mag*sin(ang)
plt.plot(xret,yret, 'k:', lw=1)
xret = mag*cos(-ang)
yret = mag*sin(-ang)
plt.plot(xret,yret,'k:', lw=1)
an_i = int(len(xret)/2.5)
an_x = xret[an_i]
an_y = yret[an_i]
plt.annotate(str(zeta), xy=(an_x, an_y), xytext=(an_x, an_y), size=7)

@murrayrm murrayrm Feb 23, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can/should we use parameters for the size? 7 seems like a magic number.


for a in linspace(0,1,10):
x = linspace(-pi/2,pi/2,200)
ang = pi*a*sin(x)
mag = exp(-pi*a*cos(x))
xret = mag*cos(ang)
yret = mag*sin(ang)
plt.plot(xret,yret,'k:', lw=1)
an_i = -1 #int(len(xret)/2.5)
an_x = xret[an_i]
an_y = yret[an_i]
num = '{:1.1f}'.format(a)
plt.annotate("$\\frac{"+num+"\pi}{T}$", xy=(an_x, an_y), xytext=(an_x, an_y), size=8)


# TODO: Implement more elegant cross-style axes. See:
# http://matplotlib.sourceforge.net/examples/axes_grid/demo_axisline_style.html
# http://matplotlib.sourceforge.net/examples/axes_grid/demo_curvelinear_grid.html
Expand Down Expand Up @@ -75,16 +106,20 @@ def pzmap(sys, Plot=True, title='Pole Zero Map'):

if (Plot):
import matplotlib.pyplot as plt

if isdtime(sys):

@murrayrm murrayrm Feb 24, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be isdtime(sys, strict=True), otherwise it will be true for systems with unspecified timebases (which could be continuous or discrete).

@Sup3rGeo Sup3rGeo Feb 25, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we are treating systems with unspecified timebases as continuous (showing s-plane omega-damping - see next comment)?

@murrayrm murrayrm Feb 25, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that if the timebase is unspecified, it can represent either a continuous-time or discrete-time system (since for many operations it doesn't matter). Because of this, if you call isdtime on a system for a system in which timebase == None, isdtime will return true. If you want something to only happen when a system is really a discrete time system, use isdtime(sys, strict=True).

zgrid(plt)

@murrayrm murrayrm Feb 24, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should make plotting the zgrid an option, rather than the default. This will give consistency with past behavior.

@Sup3rGeo Sup3rGeo Feb 25, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am actually having it similar to what we have on rlocus function, with an argument grid=False (even though IMHO plot is way better with the omega-damping grid)

I also realized we should also be using this on rlocus function, so I am making zgrid available for rlocus and making _sgrid_func() from rlocus available for pzmap.

@murrayrm murrayrm Feb 25, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this sounds like a good approach. And if we can reuse an existing function, that is even better (although I note that _sgrid_func() is not all that well commented -:).


# Plot the locations of the poles and zeros
if len(poles) > 0:
plt.scatter(real(poles), imag(poles), s=50, marker='x')
plt.scatter(real(poles), imag(poles), s=50, marker='x', facecolors='k')
if len(zeros) > 0:
plt.scatter(real(zeros), imag(zeros), s=50, marker='o',
facecolors='none')
facecolors='none', edgecolors='k')
# Add axes
#Somewhat silly workaround
plt.axhline(y=0, color='black')
plt.axvline(x=0, color='black')
plt.axhline(y=0, color='black', lw=1)
plt.axvline(x=0, color='black', lw=1)
plt.xlabel('Re')
plt.ylabel('Im')

Expand Down