-
Notifications
You must be signed in to change notification settings - Fork 438
Time response plot improvements #1018
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
Time response plot improvements #1018
Conversation
control/nlsys.py
Outdated
@@ -1327,8 +1327,8 @@ def input_output_response( | |||
|
|||
Parameters | |||
---------- | |||
sys : InputOutputSystem | |||
Input/output system to simulate. | |||
sysdata : I/O system or list of I/O systems |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is useful to include a class name here because, in that case, docs generation results in a link to documentation for the class. Unless I missed something, this should be NonlinearIOSystem
. This will work even when the type is written as "...or list of...", e.g., compare with the data
parameter of nyquist_plot
: https://python-control.readthedocs.io/en/0.10.0/generated/control.nyquist_plot.html
I will finish reviewing this tomorrow morning. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good improvements! This is ready to merge after you consider/apply my comments.
control/ctrlplot.py
Outdated
common_prefix = common_prefix[:last_space] | ||
prefix_len = len(common_prefix) | ||
|
||
# Look for a common suffice (up to a space) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Look for a common suffice (up to a space) | |
# Look for a common suffix (up to a space) |
examples/cds110_lti-systems.ipynb
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misprints:
- "to define and input/output" -> "to define an input/output"
- "response object, we can be" -> "response object, which can be"
examples/plot_gallery.py
Outdated
# used to compare what things look like between different versions of the | ||
# library. It is mainly intended for uses by developers to make sure there | ||
# are no unexpected changes in plot formats, but also has some interest | ||
# examples of htings you can plot. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# examples of htings you can plot. | |
# examples of things you can plot. |
examples/plot_gallery.py
Outdated
# Create a pdf file for storing the results | ||
from matplotlib.backends.backend_pdf import PdfPages | ||
from datetime import date | ||
git_info = os.popen('git describe').read().strip() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be considered an error if a Git repository is not found?
I ask because os.popen
will not raise an exception if the subprocess has errors, so git_info
will be an empty string and plot_gallery.py
will continue if git describe
fails. If you want an exception, I recommend check_output
:
import subprocess
git_info = subprocess.check_output(['git', 'describe'], text=True).strip()
doc/conventions.rst
Outdated
automatically compute the time vector based on the poles and zeros of | ||
system. If a list of systems is passed, a common time vector will be | ||
computed and a list of responses will be returned in the form of a | ||
:class:`TimeResponseList` object. The :func:`force_response` function can |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:class:`TimeResponseList` object. The :func:`force_response` function can | |
:class:`TimeResponseList` object. The :func:`forced_response` function can |
doc/conventions.rst
Outdated
:class:`TimeResponseList` object. The :func:`force_response` function can | ||
also take a list of systems, to which a single common input is applied. | ||
The :class:`TimeResponseList` object has a `plot()` method that will plot | ||
each of the reponses in turn, using a sequence of different colors with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
each of the reponses in turn, using a sequence of different colors with | |
each of the responses in turn, using a sequence of different colors with |
doc/conventions.rst
Outdated
For linear time invariant (LTI) systems, the :func:`impulse_response`, | ||
:func:`initial_response`, and :func:`step_response` functions will | ||
automatically compute the time vector based on the poles and zeros of | ||
system. If a list of systems is passed, a common time vector will be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
system. If a list of systems is passed, a common time vector will be | |
the system. If a list of systems is passed, a common time vector will be |
control/nlsys.py
Outdated
@@ -1317,7 +1317,7 @@ def nlsys( | |||
|
|||
|
|||
def input_output_response( | |||
sys, T, U=0., X0=0, params=None, ignore_errors=False, | |||
sysdata, T, U=0., X0=0, params=None, ignore_errors=False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the parameter name sys
to sysdata
here and in other functions modified by this pull request is good for clarity to users, but it will break existing calls of the form input_output_response(sys=ss, ...)
. As such, the next version probably should be 0.11.0 instead of 0.10.1 (#1020) to emphasize that existing code using control
may need to be updated.
This PR makes improvements to the code for plotting time responses, building on some of the improvements made for frequency response plots (#1011):
ax
keyword processing to allow arrays or lists + uniform processing for all (time and frequency) plot routines.step_response
twice and could get different default time intervals).examples/plot_gallery.py
that generates a PDF file with some standard plots to compare changes in basic plotting between PRs and releases.To get a sense of the differences, consider the following code:
Prior to this PR, the output from this code was
Note the fact that both responses are in the same color.
With this PR, the output becomes
Furthermore, you can compute a single time range using the command
which gives a single time range for both systems: