Skip to content

make it so rlocus does not always create a new figure, so it is like matlab and control.sisotool #413

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
Jul 11, 2020
Merged

make it so rlocus does not always create a new figure, so it is like matlab and control.sisotool #413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8743692
rlocus now re-uses current figure rather than always creating a new o…
sawyerbfuller Jun 3, 2020
046d432
slight change to rlocus_test to create new figure before testing root…
sawyerbfuller Jun 3, 2020
ff3b68f
rlocus changed to use current plotting axis rather than current figur…
sawyerbfuller Jun 3, 2020
7043610
rlocus: add title to root locus plot axes rather than renaming figure…
sawyerbfuller Jun 3, 2020
4745b5c
change import matplotlib as mlt to mpl for consistency
murrayrm Jul 11, 2020
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
53 changes: 23 additions & 30 deletions control/rlocus.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,10 @@
# Packages used by this module
from functools import partial
import numpy as np
import matplotlib
import matplotlib as mpl
import matplotlib.pyplot as plt
from scipy import array, poly1d, row_stack, zeros_like, real, imag
import scipy.signal # signal processing toolbox
import pylab # plotting routines
from .xferfcn import _convert_to_transfer_function
from .exception import ControlMIMONotImplemented
from .sisotool import _SisotoolUpdate
Expand All @@ -63,15 +62,16 @@
# Default values for module parameters
_rlocus_defaults = {
'rlocus.grid': True,
'rlocus.plotstr': 'b' if int(matplotlib.__version__[0]) == 1 else 'C0',
'rlocus.plotstr': 'b' if int(mpl.__version__[0]) == 1 else 'C0',
'rlocus.print_gain': True,
'rlocus.plot': True
}


# Main function: compute a root locus diagram
def root_locus(sys, kvect=None, xlim=None, ylim=None,
plotstr=None, plot=True, print_gain=None, grid=None, **kwargs):
plotstr=None, plot=True, print_gain=None, grid=None, ax=None,
**kwargs):
Copy link

@moorepants moorepants Jun 3, 2020

Choose a reason for hiding this comment

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

If you are going to add this option I think it is much better to pass in the matplotlib axes to plot on, not the figure. If you look at other libraries, that is the most common approach and offers the best way to compose multiple plot calls onto a given axes or set of axes.

Copy link
Contributor Author

@sawyerbfuller sawyerbfuller Jun 3, 2020

Choose a reason for hiding this comment

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

Makes a lot of sense to me. I did it this way partly because the existing software does a few extra things, like renaming the figure "root locus". And it instead names it "root locus 1", "root locus 2" etc if there are pre-existing "root locus" plots. I was hesitant to break that functionality.

My natural inclination would be to take that part out because it is complex, inflexible, and not in keeping with the conventions elsewhere in the library and, like you say, on plotting routines in general (e.g. pzmap re-uses current axes), but I didn't want to step on any toes.

Copy link

@moorepants moorepants Jun 3, 2020

Choose a reason for hiding this comment

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

It may be ok to remove that figure name. It isn't a strong backwards compatibility break.

But you can access the figure an axis attached to with:

In [7]: import matplotlib.pyplot as plt                                                                  

In [8]: fig, ax = plt.subplots()                                                                         


In [10]: ax.figure                                                                                       
Out[10]: <Figure size 640x480 with 1 Axes>

In [11]: ax.figure == fig                                                                                
Out[11]: True

So you can append or replace the title in the figure. This should probably have an option for it too though, i.e. modify_figure_name=True.

Copy link
Contributor Author

@sawyerbfuller sawyerbfuller Jun 3, 2020

Choose a reason for hiding this comment

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

Thanks Jason, that was super helpful, I used it to make some changes.

  • Updated the PR to make it plot in the current axis instead of the current figure.
  • removed deprecated pylab import in favor of matplotlib
  • And OK, I took out the figure (re-)naming code. Instead it now just adds a title to the plot axes.
  • Incidentally, this seems like it may have fixed the slow zooming problem referenced on lines 180-181. (at least it did for me)


"""Root locus plot

Expand All @@ -96,6 +96,8 @@ def root_locus(sys, kvect=None, xlim=None, ylim=None,
branches, calculate gain, damping and print.
grid : bool
If True plot omega-damping grid. Default is False.
ax : Matplotlib axis
axis on which to create root locus plot

Returns
-------
Expand Down Expand Up @@ -143,42 +145,33 @@ def root_locus(sys, kvect=None, xlim=None, ylim=None,
# Create the Plot
if plot:
if sisotool:
f = kwargs['fig']
ax = f.axes[1]

fig = kwargs['fig']
ax = fig.axes[1]
else:
figure_number = pylab.get_fignums()
figure_title = [
pylab.figure(numb).canvas.get_window_title()
for numb in figure_number]
new_figure_name = "Root Locus"
rloc_num = 1
while new_figure_name in figure_title:
new_figure_name = "Root Locus " + str(rloc_num)
rloc_num += 1
f = pylab.figure(new_figure_name)
ax = pylab.axes()
if ax is None:
ax = plt.gca()
fig = ax.figure
ax.set_title('Root Locus')

if print_gain and not sisotool:
f.canvas.mpl_connect(
fig.canvas.mpl_connect(
'button_release_event',
partial(_RLClickDispatcher, sys=sys, fig=f,
ax_rlocus=f.axes[0], plotstr=plotstr))

partial(_RLClickDispatcher, sys=sys, fig=fig,
ax_rlocus=fig.axes[0], plotstr=plotstr))
elif sisotool:
f.axes[1].plot(
fig.axes[1].plot(
[root.real for root in start_mat],
[root.imag for root in start_mat],
'm.', marker='s', markersize=8, zorder=20, label='gain_point')
f.suptitle(
fig.suptitle(
"Clicked at: %10.4g%+10.4gj gain: %10.4g damp: %10.4g" %
(start_mat[0][0].real, start_mat[0][0].imag,
1, -1 * start_mat[0][0].real / abs(start_mat[0][0])),
fontsize=12 if int(matplotlib.__version__[0]) == 1 else 10)
f.canvas.mpl_connect(
fontsize=12 if int(mpl.__version__[0]) == 1 else 10)
fig.canvas.mpl_connect(
'button_release_event',
partial(_RLClickDispatcher, sys=sys, fig=f,
ax_rlocus=f.axes[1], plotstr=plotstr,
partial(_RLClickDispatcher, sys=sys, fig=fig,
ax_rlocus=fig.axes[1], plotstr=plotstr,
sisotool=sisotool,
bode_plot_params=kwargs['bode_plot_params'],
tvect=kwargs['tvect']))
Expand Down Expand Up @@ -580,7 +573,7 @@ def _RLFeedbackClicksPoint(event, sys, fig, ax_rlocus, sisotool=False):
fig.suptitle(
"Clicked at: %10.4g%+10.4gj gain: %10.4g damp: %10.4g" %
(s.real, s.imag, K.real, -1 * s.real / abs(s)),
fontsize=12 if int(matplotlib.__version__[0]) == 1 else 10)
fontsize=12 if int(mpl.__version__[0]) == 1 else 10)

# Remove the previous line
_removeLine(label='gain_point', ax=ax_rlocus)
Expand Down Expand Up @@ -609,7 +602,7 @@ def _removeLine(label, ax):

def _sgrid_func(fig=None, zeta=None, wn=None):
if fig is None:
fig = pylab.gcf()
fig = plt.gcf()
ax = fig.gca()
else:
ax = fig.axes[1]
Expand Down
1 change: 1 addition & 0 deletions control/tests/rlocus_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def test_without_gains(self):
def test_root_locus_zoom(self):
"""Check the zooming functionality of the Root locus plot"""
system = TransferFunction([1000], [1, 25, 100, 0])
plt.figure()
root_locus(system)
fig = plt.gcf()
ax_rlocus = fig.axes[0]
Expand Down