From b6769f6eb8f7ac424c77dd0f5911090f3310cafc Mon Sep 17 00:00:00 2001 From: Henk van der Laak Date: Fri, 17 Feb 2023 13:58:45 +0100 Subject: [PATCH 1/5] Solve #862: bode_plot phase wrapping incorrect for multiple systems --- control/freqplot.py | 8 ++++---- control/tests/freqresp_test.py | 12 ++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/control/freqplot.py b/control/freqplot.py index d34906855..863ed1ccc 100644 --- a/control/freqplot.py +++ b/control/freqplot.py @@ -276,18 +276,18 @@ def bode_plot(syslist, omega=None, if initial_phase is None: # Start phase in the range 0 to -360 w/ initial phase = -180 # If wrap_phase is true, use 0 instead (phase \in (-pi, pi]) - initial_phase = -math.pi if wrap_phase is not True else 0 + initial_phase_value = -math.pi if wrap_phase is not True else 0 elif isinstance(initial_phase, (int, float)): # Allow the user to override the default calculation if deg: - initial_phase = initial_phase/180. * math.pi + initial_phase_value = initial_phase/180. * math.pi else: raise ValueError("initial_phase must be a number.") # Shift the phase if needed - if abs(phase[0] - initial_phase) > math.pi: + if abs(phase[0] - initial_phase_value) > math.pi: phase -= 2*math.pi * \ - round((phase[0] - initial_phase) / (2*math.pi)) + round((phase[0] - initial_phase_value) / (2*math.pi)) # Phase wrapping if wrap_phase is False: diff --git a/control/tests/freqresp_test.py b/control/tests/freqresp_test.py index 573fd6359..748ca59e1 100644 --- a/control/tests/freqresp_test.py +++ b/control/tests/freqresp_test.py @@ -375,6 +375,18 @@ def test_phase_wrap(TF, wrap_phase, min_phase, max_phase): assert(max(phase) <= max_phase) +def test_phase_wrap_multiple_systems(): + G = ctrl.zpk([],[1,1], gain=1) + + mag, phase, omega = ctrl.bode(G) + assert(np.min(phase) >= -2*np.pi) + assert(np.max(phase) <= -1*np.pi) + + mag, phase, omega = ctrl.bode((G,G)) + assert(np.min(phase) >= -2*np.pi) + assert(np.max(phase) <= -1*np.pi) + + def test_freqresp_warn_infinite(): """Test evaluation warnings for transfer functions w/ pole at the origin""" sys_finite = ctrl.tf([1], [1, 0.01]) From 2afd39769f6a42ab6cac57a22d0357294876100a Mon Sep 17 00:00:00 2001 From: Henk van der Laak Date: Fri, 17 Feb 2023 14:12:32 +0100 Subject: [PATCH 2/5] Fix wrap --- control/freqplot.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/control/freqplot.py b/control/freqplot.py index 863ed1ccc..483b4fae6 100644 --- a/control/freqplot.py +++ b/control/freqplot.py @@ -281,6 +281,9 @@ def bode_plot(syslist, omega=None, # Allow the user to override the default calculation if deg: initial_phase_value = initial_phase/180. * math.pi + else: + initial_phase_value = initial_phase + else: raise ValueError("initial_phase must be a number.") From f990f409d4c9e82de3b9909bde9fe3403c292b20 Mon Sep 17 00:00:00 2001 From: Henk van der Laak Date: Fri, 17 Feb 2023 14:38:28 +0100 Subject: [PATCH 3/5] PEP8 requires lowercase variable name --- control/tests/freqresp_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/control/tests/freqresp_test.py b/control/tests/freqresp_test.py index 748ca59e1..468ff9fdd 100644 --- a/control/tests/freqresp_test.py +++ b/control/tests/freqresp_test.py @@ -376,13 +376,13 @@ def test_phase_wrap(TF, wrap_phase, min_phase, max_phase): def test_phase_wrap_multiple_systems(): - G = ctrl.zpk([],[1,1], gain=1) + sys_unstable = ctrl.zpk([],[1,1], gain=1) - mag, phase, omega = ctrl.bode(G) + mag, phase, omega = ctrl.bode(sys_unstable) assert(np.min(phase) >= -2*np.pi) assert(np.max(phase) <= -1*np.pi) - mag, phase, omega = ctrl.bode((G,G)) + mag, phase, omega = ctrl.bode((sys_unstable, sys_unstable)) assert(np.min(phase) >= -2*np.pi) assert(np.max(phase) <= -1*np.pi) From f2878d5f941ad624efb9dc6cb624e4fff11dddd9 Mon Sep 17 00:00:00 2001 From: Henk van der Laak Date: Fri, 17 Feb 2023 14:48:10 +0100 Subject: [PATCH 4/5] Don't plot in tests... --- control/tests/freqresp_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/control/tests/freqresp_test.py b/control/tests/freqresp_test.py index 468ff9fdd..9fc52112a 100644 --- a/control/tests/freqresp_test.py +++ b/control/tests/freqresp_test.py @@ -378,11 +378,11 @@ def test_phase_wrap(TF, wrap_phase, min_phase, max_phase): def test_phase_wrap_multiple_systems(): sys_unstable = ctrl.zpk([],[1,1], gain=1) - mag, phase, omega = ctrl.bode(sys_unstable) + mag, phase, omega = ctrl.bode(sys_unstable, plot=False) assert(np.min(phase) >= -2*np.pi) assert(np.max(phase) <= -1*np.pi) - mag, phase, omega = ctrl.bode((sys_unstable, sys_unstable)) + mag, phase, omega = ctrl.bode((sys_unstable, sys_unstable), plot=False) assert(np.min(phase) >= -2*np.pi) assert(np.max(phase) <= -1*np.pi) From 4c4b3fc2a61aa817b7aadb7a61af4212b6c0ed30 Mon Sep 17 00:00:00 2001 From: Henk van der Laak Date: Fri, 17 Feb 2023 17:25:04 +0100 Subject: [PATCH 5/5] Fix regression with matplotlib 3.7.0 --- control/freqplot.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/control/freqplot.py b/control/freqplot.py index 483b4fae6..a749c3f6d 100644 --- a/control/freqplot.py +++ b/control/freqplot.py @@ -1024,9 +1024,11 @@ def _parse_linestyle(style_name, allow_false=False): # Plot the scaled sections of the curve (changing linestyle) x_scl = np.ma.masked_where(scale_mask, resp.real) y_scl = np.ma.masked_where(scale_mask, resp.imag) - plt.plot( - x_scl * (1 + curve_offset), y_scl * (1 + curve_offset), - primary_style[1], color=c, **kwargs) + if x_scl.count() >= 1 and y_scl.count() >= 1: + plt.plot( + x_scl * (1 + curve_offset), + y_scl * (1 + curve_offset), + primary_style[1], color=c, **kwargs) # Plot the primary curve (invisible) for setting arrows x, y = resp.real.copy(), resp.imag.copy() @@ -1044,10 +1046,11 @@ def _parse_linestyle(style_name, allow_false=False): # Plot the regular and scaled segments plt.plot( x_reg, -y_reg, mirror_style[0], color=c, **kwargs) - plt.plot( - x_scl * (1 - curve_offset), - -y_scl * (1 - curve_offset), - mirror_style[1], color=c, **kwargs) + if x_scl.count() >= 1 and y_scl.count() >= 1: + plt.plot( + x_scl * (1 - curve_offset), + -y_scl * (1 - curve_offset), + mirror_style[1], color=c, **kwargs) # Add the arrows (on top of an invisible contour) x, y = resp.real.copy(), resp.imag.copy()