Skip to content

Update I/O system repr() and str() #1091

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 21 commits into from
Jan 13, 2025

Conversation

murrayrm
Copy link
Member

@murrayrm murrayrm commented Jan 4, 2025

This PR provides updates to the string representations of I/O systems, including both the __repr__ and __str__ functions (used for repr(), str(), and print()) as well as LaTeX output (via _repr_html_):

  • Created a set of standard _repr_* formats for InputOutputSystem's that can be overridden in subclasses to get the most useful representations based on the system type, using method names similar to _repr_latex_ used by Jupyter notebooks:
    • _repr_info_: generates a minimal description of the I/O system in the form <SystemClass name: [input list] -> [output list][, dt=timebase]>. This is the default format that exists for all I/O systems. The inclusion of the timebase is new.
    • _repr_eval_: generates an "evaluatable" description of the I/O system, if possible (otherwise return _repr_info_()). This is used in LTI systems to show the state space matrices, numerator/denominator, or freuquency response data. This format is similar to what was done before this PR for LTI classes, but now includes the signal names (if non-generic) or the signal counts (if generic). The inclusion of the string names make sure that you can recreate the system with the proper labels. The inclusion of the string counts lets you see the number of inputs and outputs.
    • _repr_html_: generates an HTML/LaTeX description of the I/O system, if possible (otherwise return _repr_info_()). This method is picked up by Jupyter notebooks to display the value of a variable and replaces _repr_latex_ used in prior versions. This is similar to what was present before, except that now there is a "header" that shows the system name and signal names before showing a LaTeX representation of the state space matrices and/or transfer functions (this required changing _repr_latex_ to _repr_html_). Note that since the timebase information is included in the header, it is no longer included in the LaTeX formatted code.
  • Created a new function iosys_repr (similar to np.array_repr) that allows different representations of I/O systems via a format parameter ('info', 'eval', or 'latex').
  • The transfer function display_format parameter is now handled in a dynamic fashion. If it is set to None (default) on system creation, the display style will follow config.defaults['xferfcn.display_format']. If set to poly or zpk on system creation, that overrides the default.
  • Updated the str() (print) representation of InputOutputSystems to include dt after the state/input/output names (suppressed if it matches the default dt).
  • Updated the str() (print) representation for NonlinearIOSystems to include list of parameter labels.
  • Updated the str() (print) representation for InterconnectedSystems to include the list of subsystems (in 'info' form) as well summaries of the connections and outputs matrix (might be an eventual replacement for connection_table).
  • All representations now apply np.printoptions to system arrays before generating strings, so you get consistent formatting across representations (including latex) and you can use things like suppress to display near-zero numbers as '0'.
  • Fixed up various line spacings in str() (print) to make things consistent across the various I/O system classes, including adding some indentation for transfer functions and MIMO systems (see screen shots below).
  • Added examples/repr_gallery.ipynb and exampes/repr_gallery.py files to show all of the various output formats and options (with dashed lines at the start and end so you can see any extraneous blank lines).
  • Added config.defaults options 'iosys.repr_format' and 'iosys.repr_show_count' to control representation formats.
  • The combine_tf function now accepts system and signal name keywords. (This is not strictly related to the main subject of this PR, but I needed since I had to set the name of the system in order for doctests to work.)
  • Added the ability of config.defaults to be called as a context manager, so that you can temporarily change default parameters (this is used in examples/repr_gallery.py). (This is more general than this PR, but I used it extensively in repr_gallery).
  • Updated docstrings and unit tests.

A detailed set of before and after views are attached as PDFs, generated by the new examples/repr_gallery.ipynb and exampes/repr_gallery.py files:

Some highlights (pulled from repr_gallery):

  • State space system, repr(), 'eval' format:
    StateSpace: sys_ss, dt=0:
    -------------------------
    StateSpace(
    array([[ 0., 1.],
    [-4., -5.]]),
    array([[0.],
    [1.]]),
    array([[-1., 1.]]),
    array([[0.]]),
    name='sys_ss', states=2, outputs=1, inputs=1)
    ----
    
  • Discrete time, state space system, repr(), 'eval' format:
    StateSpace: sys_dss, dt=0.1:
    ----------------------------
    StateSpace(
    array([[ 0.98300988, 0.07817246],
    [-0.31268983, 0.59214759]]),
    array([[0.00424753],
    [0.07817246]]),
    array([[-1., 1.]]),
    array([[0.]]),
    dt=0.1,
    name='sys_dss', states=2, outputs=1, inputs=1)
    ----
    
  • MIMO transfer function, repr(), 'eval' format:
    TransferFunction: sys_mtf_zpk, dt=0:
    ------------------------------------
    TransferFunction(
    [[array([ 1., -1.]), array([0.])],
    [array([1, 0]), array([1, 0])]],
    [[array([1., 5., 4.]), array([1.])],
    [array([1]), array([1, 2, 1])]],
    name='sys_mtf_zpk', outputs=2, inputs=2)
    ----
    
  • Discrete time, state space system, repr(), ,'info' format:
    TransferFunction: sys_dss_poly, dt=0.1:
    ---------------------------------------
    <TransferFunction sys_dss_poly: ['u[0]'] -> ['y[0]'], dt=0.1>
    ----
    
  • Discrete time, state space systems, str():
    <StateSpace>: sys_dss
    Inputs (1): ['u[0]']
    Outputs (1): ['y[0]']
    States (2): ['x[0]', 'x[1]']
    dt = 0.1
    
    A = [[ 0.98300988 0.07817246]
         [-0.31268983 0.59214759]]
    
    B = [[0.00424753]
         [0.07817246]]
    
    C = [[-1. 1.]]
    
    D = [[0.]]
    ----
    
  • Nonlinear I/O system, str():
    NonlinearIOSystem: sys_nl, dt=0:
    --------------------------------
    <NonlinearIOSystem>: sys_nl
    Inputs (1): ['u[0]']
    Outputs (1): ['y[0]']
    States (2): ['x[0]', 'x[1]']
    Parameters: ['a', 'b']
    
    Update: <function nl_update at 0x1258ed800>
    Output: <function nl_output at 0x1258ed8a0>
    
  • Linear interconnected system, str():
    LinearICSystem: sys_ic, dt=0:
    -----------------------------
    <LinearICSystem>: sys_ic
    Inputs (2): ['r[0]', 'r[1]']
    Outputs (2): ['y[0]', 'y[1]']
    States (2): ['proc_x[0]', 'proc_x[1]']
    
    Subsystems (2):
    * <StateSpace proc: ['u[0]', 'u[1]'] -> ['y[0]', 'y[1]']>
    * <StateSpace ctrl: ['u[0]', 'u[1]'] -> ['y[0]', 'y[1]'], dt=None>
    
    Connections:
    * proc.u[0] <- ctrl.y[0]
    * proc.u[1] <- ctrl.y[1]
    * ctrl.u[0] <- -proc.y[0] + r[0]
    * ctrl.u[1] <- -proc.y[1] + r[1]
    
    Outputs:
    * y[0] <- proc.y[0]
    * y[1] <- proc.y[1]
    
    A = [[-2. 3.]
        [-1. -5.]]
    
    B = [[-2. 0.]
        [ 0. -3.]]
    
    C = [[-1. 1.]
        [ 1. 0.]]
    
    D = [[0. 0.]
        [0. 0.]]
    ----
    

@coveralls
Copy link

coveralls commented Jan 4, 2025

Coverage Status

coverage: 94.686% (-0.1%) from 94.781%
when pulling 0f0fad0 on murrayrm:iosys_repr-07Dec2024
into a1791c9 on python-control:main.

Copy link
Contributor

@sawyerbfuller sawyerbfuller left a comment

Choose a reason for hiding this comment

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

Nice PR. Showing signal names by default in notebooks will help my students/users catch a lot of bugs.

Is the different format for the inputs for stateless systems u0 , u1 vs u[0] , u[1] for other systems a bug or a feature? Maybe a bug that this PR uncovered.

@murrayrm
Copy link
Member Author

murrayrm commented Jan 4, 2025

Is the different format for the inputs for stateless systems u0 , u1 vs u[0] , u[1] for other systems a bug or a feature? Maybe a bug that this PR uncovered.

The input names appear for that example because the inputs were given names when the system was created(and so they show up explicitly to preserve that naming).

@slivingston slivingston self-requested a review January 5, 2025 03:58
Copy link
Member Author

@murrayrm murrayrm left a comment

Choose a reason for hiding this comment

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

A bit odd for me to be reviewing my own PR, but this is a pretty big PR and it built on top of another one, so I wanted to do one last check. I found a couple of things to look into.

Comment on lines 336 to 339
# Verions 0.10.2
if major == 0 and minor <= 10 and patch < 2:
set_defaults('iosys', repr_format='eval')

Copy link
Member Author

Choose a reason for hiding this comment

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

Current release also uses eval as default, so this may not be needed. We could set iosys.show_count to False here, to match v0.10.1, but it seems like including the counts is never something that is going to cause a problem in running code.

Comment on lines +270 to +274
escape_chars = {
'$': r'\$',
'<': '&lt;',
'>': '&gt;',
Copy link
Member Author

Choose a reason for hiding this comment

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

It might look better to change the -> in the I/O system representation into $\to$? (This would require a bit more complex checking in this function.)

Copy link
Member Author

@murrayrm murrayrm Jan 8, 2025

Choose a reason for hiding this comment

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

I left this alone but added a comment to enhance this in a future release.

control/iosys.py Outdated

Notes
-----
By default, the representation for an input/output is set to 'info'.
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is now 'eval'; see top of file.

control/nlsys.py Outdated
@@ -154,9 +154,13 @@ def __init__(self, updfcn, outfcn=None, params=None, **kwargs):
self._current_params = {} if params is None else params.copy()

def __str__(self):
return f"{InputOutputSystem.__str__(self)}\n\n" + \
out = f"{InputOutputSystem.__str__(self)}"
if len(self.params) > 1:
Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't this be > 0?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed and added a unit test that exhibits the (now corrected) error.

Comment on lines +1482 to +1488
# Use StateSpace.__call__ to evaluate at a given complex value
def __call__(self, *args, **kwargs):
return StateSpace.__call__(self, *args, **kwargs)
Copy link
Member Author

Choose a reason for hiding this comment

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

Since this was apparently incorrect before, is there a missing unit test?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed (indentation was wrong) and added unit test. Turns out the original version worked because it picked the right parent class (StateSpace versus NonlinearIO)

Copy link
Member

@slivingston slivingston left a comment

Choose a reason for hiding this comment

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

I need 1 day more to finish reviewing. Sending the small misprints I noticed thus far.

# of representations (__repr__, __str__) for those systems that can be
# used to compare different versions of python-control. It is mainly
# intended for uses by developers to make sure there are no unexpected
# changes in representation formats, but also has some interest
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# changes in representation formats, but also has some interest
# changes in representation formats, but also has some interesting

Copy link
Member

Choose a reason for hiding this comment

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

The suggested change applies to repr_gallery.ipynb, too.

sys_gtf = ct.tf([1], [1, 0])
syslist += [sys_tf, sys_dtf, sys_gtf]

# MIMO transfer function (continous time only)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# MIMO transfer function (continous time only)
# MIMO transfer function (continuous time only)

name='sys_mtf_zpk', display_format='zpk')
syslist += [sys_mtf]

# Frequency response data (FRD) system (continous and discrete time)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Frequency response data (FRD) system (continous and discrete time)
# Frequency response data (FRD) system (continuous and discrete time)

@@ -311,6 +333,10 @@ def use_legacy_defaults(version):
#
reset_defaults() # start from a clean slate

# Verions 0.10.2
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Verions 0.10.2
# Version 0.10.2

@@ -19,7 +19,7 @@
# Basic test of nlsys()
def test_nlsys_basic():
def kincar_update(t, x, u, params):
l = params.get('l', 1) # wheelbase
l = params['l'] # wheelbase
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
l = params['l'] # wheelbase
l = params['l'] # wheelbase

1 space more to align with "x velocity" on line 24.

sys.repr_format = format
out = repr(sys)
if format == 'eval':
assert re.search(expected, out) != None
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
assert re.search(expected, out) != None
assert re.search(expected, out) is not None

Preferred style (in PEP 8) is is not None instead of != None. This comment applies to != None on lines 2370 and 2348, too.

sys = ct.ss([[1]], [[1]], [[1]], [[0]])
new = eval(repr(sys))
for attr in ['A', 'B', 'C', 'D']:
assert getattr(sys, attr) == getattr(sys, attr)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
assert getattr(sys, attr) == getattr(sys, attr)
assert getattr(sys, attr) == getattr(new, attr)

@murrayrm
Copy link
Member Author

@slivingston Thanks for the detailed review. Let me know if additional comments are coming, otherwise I'll go head and merge.

@murrayrm murrayrm force-pushed the iosys_repr-07Dec2024 branch from b44d5ba to 0f0fad0 Compare January 13, 2025 01:29
@slivingston
Copy link
Member

@slivingston Thanks for the detailed review. Let me know if additional comments are coming, otherwise I'll go head and merge.

I will do one more look now and wrap it up within 2 hours.

@murrayrm murrayrm merged commit ec7dc8a into python-control:main Jan 13, 2025
23 checks passed
@murrayrm murrayrm mentioned this pull request Jan 13, 2025
6 tasks
@murrayrm murrayrm added this to the 0.10.2 milestone Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants