Skip to content

fix rlocus timeout due to inefficient _default_wn calculation #527

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 1 commit into from
Feb 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 18 additions & 10 deletions control/rlocus.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,8 +646,9 @@ def _sgrid_func(fig=None, zeta=None, wn=None):
else:
ax = fig.axes[1]

# Get locator function for x-axis tick marks
# Get locator function for x-axis, y-axis tick marks
xlocator = ax.get_xaxis().get_major_locator()
ylocator = ax.get_yaxis().get_major_locator()

# Decide on the location for the labels (?)
ylim = ax.get_ylim()
Expand Down Expand Up @@ -690,7 +691,7 @@ def _sgrid_func(fig=None, zeta=None, wn=None):
# omega-constant lines
angles = np.linspace(-90, 90, 20) * np.pi/180
if wn is None:
wn = _default_wn(xlocator(), ylim)
wn = _default_wn(xlocator(), ylocator())

for om in wn:
if om < 0:
Expand Down Expand Up @@ -746,7 +747,7 @@ def _default_zetas(xlim, ylim):
return zeta.tolist()


def _default_wn(xloc, ylim):
def _default_wn(xloc, yloc, max_lines=7):
"""Return default wn for root locus plot

This function computes a list of natural frequencies based on the grid
Expand All @@ -758,23 +759,30 @@ def _default_wn(xloc, ylim):
List of x-axis tick values
ylim : array_like
List of y-axis limits [min, max]
max_lines : int, optional
Maximum number of frequencies to generate (default = 7)

Returns
-------
wn : list
List of default natural frequencies for the plot

"""
sep = xloc[1]-xloc[0] # separation between x-ticks

# Decide whether to use the x or y axis for determining wn
if yloc[-1] / sep > max_lines*10:
# y-axis scale >> x-axis scale
wn = yloc # one frequency per y-axis tick mark
else:
wn = xloc # one frequency per x-axis tick mark

wn = xloc # one frequency per x-axis tick mark
sep = xloc[1]-xloc[0] # separation between ticks

# Insert additional frequencies to span the y-axis
while np.abs(wn[0]) < ylim[1]:
wn = np.insert(wn, 0, wn[0]-sep)
# Insert additional frequencies to span the y-axis
while np.abs(wn[0]) < yloc[-1]:
wn = np.insert(wn, 0, wn[0]-sep)

# If there are too many values, cut them in half
while len(wn) > 7:
while len(wn) > max_lines:
wn = wn[0:-1:2]

return wn
Expand Down
29 changes: 29 additions & 0 deletions control/tests/rlocus_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from numpy.testing import assert_array_almost_equal
import pytest

import control as ct
from control.rlocus import root_locus, _RLClickDispatcher
from control.xferfcn import TransferFunction
from control.statesp import StateSpace
Expand Down Expand Up @@ -74,3 +75,31 @@ def test_root_locus_zoom(self):

assert_array_almost_equal(zoom_x, zoom_x_valid)
assert_array_almost_equal(zoom_y, zoom_y_valid)

def test_rlocus_default_wn(self):
"""Check that default wn calculation works properly"""
#
# System that triggers use of y-axis as basis for wn (for coverage)
#
# This system generates a root locus plot that used to cause the
# creation (and subsequent deletion) of a large number of natural
# frequency contours within the `_default_wn` function in `rlocus.py`.
# This unit test makes sure that is fixed by generating a test case
# that will take a long time to do the calculation (minutes).
#
import scipy as sp
import signal

# Define a system that exhibits this behavior
sys = ct.tf(*sp.signal.zpk2tf(
[-1e-2, 1-1e7j, 1+1e7j], [0, -1e7j, 1e7j], 1))

# Set up a timer to catch execution time
def signal_handler(signum, frame):
raise Exception("rlocus took too long to complete")
signal.signal(signal.SIGALRM, signal_handler)

# Run the command and reset the alarm
signal.alarm(2) # 2 second timeout
ct.root_locus(sys)
signal.alarm(0) # reset the alarm