Skip to content

Update nyquist_plot to call nyquist_response correctly #1096

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 2 commits into from
Jan 16, 2025
Merged

Update nyquist_plot to call nyquist_response correctly #1096

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6db1a1c
update nyquist_plot to call nyquist_response with all args + unit tests
murrayrm Jan 14, 2025
54c4fd9
fix small typos per @slivingston review
murrayrm Jan 16, 2025
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
26 changes: 16 additions & 10 deletions control/freqplot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1162,7 +1162,7 @@ def plot(self, *args, **kwargs):
def nyquist_response(
sysdata, omega=None, omega_limits=None, omega_num=None,
return_contour=False, warn_encirclements=True, warn_nyquist=True,
_check_kwargs=True, **kwargs):
_kwargs=None, _check_kwargs=True, **kwargs):
"""Nyquist response for a system.

Computes a Nyquist contour for the system over a (optional) frequency
Expand Down Expand Up @@ -1263,21 +1263,28 @@ def nyquist_response(
>>> lines = response.plot()

"""
# Create unified list of keyword arguments
if _kwargs is None:
_kwargs = kwargs
else:
# Use existing dictionary, to keep track of processed keywords
_kwargs |= kwargs

# Get values for params
omega_num_given = omega_num is not None
omega_num = config._get_param('freqplot', 'number_of_samples', omega_num)
indent_radius = config._get_param(
'nyquist', 'indent_radius', kwargs, _nyquist_defaults, pop=True)
'nyquist', 'indent_radius', _kwargs, _nyquist_defaults, pop=True)
encirclement_threshold = config._get_param(
'nyquist', 'encirclement_threshold', kwargs,
'nyquist', 'encirclement_threshold', _kwargs,
_nyquist_defaults, pop=True)
indent_direction = config._get_param(
'nyquist', 'indent_direction', kwargs, _nyquist_defaults, pop=True)
'nyquist', 'indent_direction', _kwargs, _nyquist_defaults, pop=True)
indent_points = config._get_param(
'nyquist', 'indent_points', kwargs, _nyquist_defaults, pop=True)
'nyquist', 'indent_points', _kwargs, _nyquist_defaults, pop=True)

if _check_kwargs and kwargs:
raise TypeError("unrecognized keywords: ", str(kwargs))
if _check_kwargs and _kwargs:
raise TypeError("unrecognized keywords: ", str(_kwargs))

# Convert the first argument to a list
syslist = sysdata if isinstance(sysdata, (list, tuple)) else [sysdata]
Expand Down Expand Up @@ -1782,15 +1789,14 @@ def _parse_linestyle(style_name, allow_false=False):
if all([isinstance(
sys, (StateSpace, TransferFunction, FrequencyResponseData))
for sys in data]):
# Get the response, popping off keywords used there
# Get the response; pop explicit keywords here, kwargs in _response()
nyquist_responses = nyquist_response(
data, omega=omega, return_contour=return_contour,
omega_limits=kwargs.pop('omega_limits', None),
omega_num=kwargs.pop('omega_num', None),
warn_encirclements=kwargs.pop('warn_encirclements', True),
warn_nyquist=kwargs.pop('warn_nyquist', True),
indent_radius=kwargs.pop('indent_radius', None),
_check_kwargs=False, **kwargs)
_kwargs=kwargs, _check_kwargs=False)
else:
nyquist_responses = data

Expand Down
79 changes: 79 additions & 0 deletions control/tests/response_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# response_test.py - test response/plot design pattern
# RMM, 13 Jan 2025
#
# The standard pattern for control plots is to call a _response() or _map()
# function and then use the plot() method. However, it is also allowed to
# call the _plot() function directly, in which case the _response()/_map()
# function is called internally.
#
# If there are arguments that are allowed in _plot() that need to be
# processed by _response(), then we need to make sure that arguments are
# properly passed from _plot() to _response(). The unit tests in this file
# make sure that this functionality is implemented properly across all
# *relevan* _response/_map/plot pairs.

@slivingston slivingston Jan 14, 2025

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.

Suggested change
# *relevan* _response/_map/plot pairs.
# *relevant* _response/_map/plot pairs.

#
# Response/map function Plotting function Comments
# --------------------- ----------------- --------
# describing_function_response describing_function_plot no passthru args
# forced_response time_response_plot no passthru args
# frequency_response bode_plot included below
# frequency_response nichols_plot included below
# gangof4_response gangof4_plot included below
# impulse_response time_response_plot no passthru args
# initial_response time_response_plot no passthru args
# input_output_response time_response_plot no passthru args
# nyquist_response nyquist_plot included below
# pole_zero_map pole_zero_plot no passthru args
# root_locus_map pole_zero_plot included below

@slivingston slivingston Jan 15, 2025

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.

Suggested change
# root_locus_map pole_zero_plot included below
# root_locus_map root_locus_plot included below

# singular_values_response singular_values_plot included below
# step_response time_response_plot no passthru args

import matplotlib.pyplot as plt
import numpy as np
import pytest

import control as ct


# List of parameters that should be processed by response function
@pytest.mark.parametrize("respfcn, plotfcn, respargs", [
(ct.frequency_response, ct.bode_plot,
{'omega_limits': [1e-2, 1e2], 'omega_num': 50, 'Hz': True}),
(ct.frequency_response, ct.bode_plot, {'omega': np.logspace(2, 2)}),
(ct.frequency_response, ct.nichols_plot, {'omega': np.logspace(2, 2)}),
(ct.gangof4_response, ct.gangof4_plot, {'omega': np.logspace(2, 2)}),
(ct.gangof4_response, ct.gangof4_plot,
{'omega_limits': [1e-2, 1e2], 'omega_num': 50, 'Hz': True}),
(ct.nyquist_response, ct.nyquist_plot,
{'indent_direction': 'right', 'indent_radius': 0.1, 'indent_points': 100,
'omega_num': 50, 'warn_nyquist': False}),
(ct.root_locus_map, ct.root_locus_plot, {'gains': np.linspace(1, 10, 5)}),
(ct.singular_values_response, ct.singular_values_plot,
{'omega_limits': [1e-2, 1e2], 'omega_num': 50, 'Hz': True}),
(ct.singular_values_response, ct.singular_values_plot,
{'omega': np.logspace(2, 2)}),
])
@pytest.mark.usefixtures('mplcleanup')
def test_response_plot(respfcn, plotfcn, respargs):
if respfcn is ct.gangof4_response:
# Two arguments required
args = (ct.rss(4, 1, 1, strictly_proper=True), ct.rss(1, 1, 1))
else:
# Single argument is enough
args = (ct.rss(4, 1, 1, strictly_proper=True), )

# Standard calling pattern - generate response, then plot
plt.figure()
resp = respfcn(*args, **respargs)
if plotfcn is ct.nichols_plot:
cplt_resp = resp.plot(plot_type='nichols')
else:
cplt_resp = resp.plot()

# Alternative calling pattern - call plotting function directly
plt.figure()
cplt_plot = plotfcn(*args, **respargs)

# Make sure the plots have the same elements
assert cplt_resp.lines.shape == cplt_plot.lines.shape
assert cplt_resp.axes.shape == cplt_plot.axes.shape