8000 Cover more of root_locus: dtime and sisotool by bnavigator · Pull Request #617 · python-control/python-control · GitHub
[go: up one dir, main page]
Skip to content

Cover more of root_locus: dtime and sisotool #617

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
merged 3 commits into from
Jun 16, 2021
Merged

Cover more of root_locus: dtime and sisotool #617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca302d6
add coverage for rlocus with dtime, grid and sisotool
bnavigator Apr 29, 2021
39dee6f
simplify grid and sisotool logic
bnavigator Apr 29, 2021
fae38af
more coverage
bnavigator Apr 29, 2021
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
11 changes: 3 additions & 8 deletions control/rlocus.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def root_locus(sys, kvect=None, xlim=None, ylim=None,
print_gain = config._get_param(
'rlocus', 'print_gain', print_gain, _rlocus_defaults)

sys_loop = sys if sys.issiso() else sys[0,0]
sys_loop = sys if sys.issiso() else sys[0, 0]

# Convert numerator and denominator to polynomials if they aren't
(nump, denp) = _systopoly1d(sys_loop)
Expand Down Expand Up @@ -232,16 +232,11 @@ def root_locus(sys, kvect=None, xlim=None, ylim=None,
ax.set_ylim(ylim)

# Draw the grid
if grid and sisotool:
if isdtime(sys, strict=True):
zgrid(ax=ax)
else:
_sgrid_func(fig=fig)
elif grid:
if grid:
if isdtime(sys, strict=True):
zgrid(ax=ax)
else:
_sgrid_func()
_sgrid_func(fig=fig if sisotool else None)
Copy link
Contributor

Choose a reason for hiding this comment

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

Any idea why there are two different sgrid functions? there is this one and there is grid.sgrid. Maybe a different PR, but would be good to unify these and get the best of each of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently _sgrid_func() was removed in #193 but reintroduced through some merge gone wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

908fabe

else:
ax.axhline(0., linestyle=':', color='k', linewidth=.75, zorder=-20)
ax.axvline(0., linestyle=':', color='k', linewidth=.75, zorder=-20)
Expand Down
5 changes: 0 additions & 5 deletions control/setup.py
View file
Open in desktop

This file was deleted.

51 changes: 43 additions & 8 deletions control/tests/rlocus_test.py
View file
Open in desktop
39 changes: 11 additions & 28 deletions control/tests/sisotool_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,25 @@
from control.bdalg import feedback


@pytest.mark.usefixtures("mplcleanup")
class TestRootLocus:
"""These are tests for the feedback function in rlocus.py."""

@pytest.fixture(params=[(TransferFunction, ([1, 2], [1, 2, 3])),
(StateSpace, ([[1., 4.], [3., 2.]],
[[1.], [-4.]],
[[1., 0.]], [[0.]]))],
ids=["tf", "ss"])
@pytest.fixture(params=[pytest.param((sysclass, sargs + (dt, )),
id=f"{systypename}-{dtstring}")
for sysclass, systypename, sargs in [
(TransferFunction, 'TF', ([1, 2],
[1, 2, 3])),
(StateSpace, 'SS', ([[1., 4.], [3., 2.]],
[[1.], [-4.]],
[[1., 0.]],
[[0.]])),
]
for dt, dtstring in [(0, 'ctime'),
(True, 'dtime')]
])
def sys(self, request):
"""Return some simple LTI system for testing"""
"""Return some simple LTI systems for testing"""
# avoid construction during collection time: prevent unfiltered
# deprecation warning
sysfn, args = request.param
Expand All @@ -37,7 +46,7 @@ def check_cl_poles(self, sys, pole_list, k_list):
np.testing.assert_array_almost_equal(poles, poles_expected)

def testRootLocus(self, sys):
"""Basic root locus plot"""
"""Basic root locus (no plot)"""
klist = [-1, 0, 1]

roots, k_out = root_locus(sys, klist, plot=False)
Expand All @@ -49,6 +58,33 @@ def test_without_gains(self, sys):
roots, kvect = root_locus(sys, plot=False)
self.check_cl_poles(sys, roots, kvect)

@pytest.mark.parametrize('grid', [None, True, False])
def test_root_locus_plot_grid(self, sys, grid):
rlist, klist = root_locus(sys, grid=grid)
ax = plt.gca()
n_gridlines = sum([int(line.get_linestyle() in [':', 'dotted',
'--', 'dashed'])
for line in ax.lines])
if grid is False:
assert n_gridlines == 2
else:
assert n_gridlines > 2
# TODO check validity of grid

def test_root_locus_warnings(self):
sys = TransferFunction([1000], [1, 25, 100, 0])
with pytest.warns(FutureWarning, match="Plot.*deprecated"):
rlist, klist = root_locus(sys, Plot=True)
with pytest.warns(FutureWarning, match="PrintGain.*deprecated"):
rlist, klist = root_locus(sys, PrintGain=True)

def test_root_locus_neg_false_gain_nonproper(self):
""" Non proper TranferFunction with negative gain: Not implemented"""
with pytest.raises(ValueError, match="with equal order"):
root_locus(TransferFunction([-1, 2], [1, 2]))

# TODO: cover and validate negative false_gain branch in _default_gains()

def test_root_locus_zoom(self):
"""Check the zooming functionality of the Root locus plot"""
system = TransferFunction([1000], [1, 25, 100, 0])
Expand Down Expand Up @@ -96,4 +132,3 @@ def test_rlocus_default_wn(self):
[-1e-2, 1-1e7j, 1+1e7j], [0, -1e7j, 1e7j], 1))

ct.root_locus(sys)

Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,10 @@ class TestSisotool:
"""These are tests for the sisotool in sisotool.py."""

@pytest.fixture
def sys(self):
def tsys(self, request):
"""Return a generic SISO transfer function"""
return TransferFunction([1000], [1, 25, 100, 0])

@pytest.fixture
def sysdt(self):
"""Return a generic SISO transfer function"""
return TransferFunction([1000], [1, 25, 100, 0], True)
dt = getattr(request, 'param', 0)
return TransferFunction([1000], [1, 25, 100, 0], dt)

@pytest.fixture
def sys222(self):
Expand All @@ -50,8 +46,8 @@ def sys221(self):
D221 = [[1., -1.]]
return StateSpace(A222, B222, C221, D221)

def test_sisotool(self, sys):
sisotool(sys, Hz=False)
def test_sisotool(self, tsys):
sisotool(tsys, Hz=False)
fig = plt.gcf()
ax_mag, ax_rlocus, ax_phase, ax_step = fig.axes[:4]

Expand Down Expand Up @@ -89,7 +85,7 @@ def test_sisotool(self, sys):
event = type('test', (object,), {'xdata': 2.31206868287,
'ydata': 15.5983051046,
'inaxes': ax_rlocus.axes})()
_RLClickDispatcher(event=event, sys=sys, fig=fig,
_RLClickDispatcher(event=event, sys=tsys, fig=fig,
ax_rlocus=ax_rlocus, sisotool=True, plotstr='-',
bode_plot_params=bode_plot_params, tvect=None)

Expand Down Expand Up @@ -118,37 +114,24 @@ def test_sisotool(self, sys):
assert_array_almost_equal(
ax_step.lines[0].get_data()[1][:10], step_response_moved, 4)

def test_sisotool_tvect(self, sys):
@pytest.mark.parametrize('tsys', [0, True],
indirect=True, ids=['ctime', 'dtime'])
def test_sisotool_tvect(self, tsys):
# test supply tvect
tvect = np.linspace(0, 1, 10)
sisotool(sys, tvect=tvect)
sisotool(tsys, tvect=tvect)
fig = plt.gcf()
ax_rlocus, ax_step = fig.axes[1], fig.axes[3]

# Move the rootlocus to another point and confirm same tvect
event = type('test', (object,), {'xdata': 2.31206868287,
'ydata': 15.5983051046,
'inaxes': ax_rlocus.axes})()
_RLClickDispatcher(event=event, sys=sys, fig=fig,
_RLClickDispatcher(event=event, sys=tsys, fig=fig,
ax_rlocus=ax_rlocus, sisotool=True, plotstr='-',
bode_plot_params=dict(), tvect=tvect)
assert_array_almost_equal(tvect, ax_step.lines[0].get_data()[0])

def test_sisotool_tvect_dt(self, sysdt):
# test supply tvect
tvect = np.linspace(0, 1, 10)
sisotool(sysdt, tvect=tvect)
fig = plt.gcf()
ax_rlocus, ax_step = fig.axes[1], fig.axes[3]

# Move the rootlocus to another point and confirm same tvect
event = type('test', (object,), {'xdata': 2.31206868287,
'ydata': 15.5983051046,
'inaxes': ax_rlocus.axes})()
_RLClickDispatcher(event=event, sys=sysdt, fig=fig,
ax_rlocus=ax_rlocus, sisotool=True, plotstr='-',
bode_plot_params=dict(), tvect=tvect)
assert_array_almost_equal(tvect, ax_step.lines[0].get_data()[0])

def test_sisotool_mimo(self, sys222, sys221):
# a 2x2 should not raise an error:
Expand Down
1 change: 0 additions & 1 deletion setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,3 @@ universal=1
addopts = -ra
filterwarnings =
error:.*matrix subclass:PendingDeprecationWarning

0