print a connection table for interconnected systems #925

sawyerbfuller · 2023-07-24T21:26:37Z

This PR prints out a table of each signal name, where it comes from (source), and where it goes (destination). It is intended primarily for systems that have been connected implicitly, because in that case all of the signal names have been defined. It doesn't really work for systems that have been connected explicitly, because their signal names may not be unique

It was is inspired by a similar table printout in bdsim and the need to have a means to debug interconnections of many systems.

Example:

        >>> P = ct.ss(1,1,1,0, inputs='u', outputs='y')
        >>> C = ct.tf(10, [.1, 1], inputs='e', outputs='u')
        >>> L = ct.interconnect([C, P], inputs='e', outputs='y')
        >>> L.connection_table()
        signal    | source                  | destination
        --------------------------------------------------------------
        e         | input                   | system 0
        u         | system 0                | system 1
        y         | system 1                | output

(edit: in the code above, signal_table has been changed to connection_table)
Remarks:

it does not list systems by their name by default because the because default system names (e.g. sys[17]) are not very descriptive or helpful when you're connecting systems implicitly with signal names. instead the table just references them by the order they appear in the interconnect list, with an option to print names instead.
it would be convenient if instead systems were listed by the name of the variable referencing them, e.g. if the first row looked like e | input | C , but this is not straightforward in Python because it is possible to have many variable names pointing to the same object in memory.
Maybe this could be achieved using a change to the library that automatically populates the 'name' field in new systems with the variable name it is first assigned to, but I am not sure if this is possible or Pythonic
this PR also includes a couple of docstring cleanups to remove references to the namedio class

coveralls · 2023-07-24T23:07:38Z

coverage: 94.969% (+0.001%) from 94.968% when pulling 5c23f1a on sawyerbfuller:connections into 42c6fb1 on python-control:main.

murrayrm · 2023-07-30T16:56:05Z

This looks very useful for sorting out what is connected to what. A couple of questions and comments:

What happens if the list of systems is longer than 32 characters. Is there a way to either truncate the field (perhaps end in "...") or adjust the spacing variable to make sure that the table stays lined up?
Is there a way to generate a warning if the system was not generated using common signal names?
A more general functionality seems like it would be possible by looking through the connect_map, input_map, and output_map arrays, which describe how everything is connected.

As an experiment, I tried modifying the example above to use a more typical form with a reference input:

P = ct.ss(1,1,1,0, inputs='u', outputs='y')
C = ct.tf(10, [.1, 1], inputs='e', outputs='u')
sumblk = ct.summing_junction(inputs=['r', '-y'], outputs='e')
T = ct.interconnect([C, P, sumblk], inputs='r', outputs='y')
T.signal_table()

which gives

signal    | source                        | destination
--------------------------------------------------------------------------
e         | system 2                      | system 0                      
y         | system 1                      | output, system 2              
r         | input                         | system 2                      
u         | system 0                      | system 1

That looks right to me, but it it might be nice to see the fact that y goes into system 2 (the summing junction) with a negative gain.

sawyerbfuller · 2023-08-02T22:01:57Z

This looks very useful for sorting out what is connected to what. A couple of questions and comments:
* What happens if the list of systems is longer than 32 characters.  Is there a way to either truncate the field (perhaps end in "...") or adjust the `spacing` variable to make sure that the table stays lined up?

Right now it just breaks alignment. I think a good solution might be to add an elipsis as you suggest, and to allow the user to set the spacing parameter. update forthcoming.

* Is there a way to generate a warning if the system was not generated using common signal names?

* A more general functionality seems like it would be possible by looking through the `connect_map`, `input_map`, and `output_map` arrays, which describe how everything is connected.

On further thought, I would propose the following:

change the name of this function to be more generic, such as "connection_table", rather than "signal_table", allowing for a future PR that perhaps produces a different kind of table for explicitly-connected systems.
which would pave the way for a future PR geared toward explicitly-connected systems that perhaps uses those maps. OTOH I think that such interconnections might better be presented in a table that puts the system name first, followed by its input and output systems. Perhaps a flag could be added to to the InterconnectedSystem class, specifying whether it was connected implicitly or explicitly, and choose the table type based on that.
(edit) add in a warning for the time being

As an experiment, I tried modifying the example above to use a more typical form with a reference input:

P = ct.ss(1,1,1,0, inputs='u', outputs='y')
C = ct.tf(10, [.1, 1], inputs='e', outputs='u')
sumblk = ct.summing_junction(inputs=['r', '-y'], outputs='e')
T = ct.interconnect([C, P, sumblk], inputs='r', outputs='y')
T.signal_table()

which gives

signal    | source                        | destination
--------------------------------------------------------------------------
e         | system 2                      | system 0                      
y         | system 1                      | output, system 2              
r         | input                         | system 2                      
u         | system 0                      | system 1

That looks right to me, but it it might be nice to see the fact that y goes into system 2 (the summing junction) with a negative gain.

Great. I agree this could be useful. Open to it going into a future PR? I am not sure this information is stored anywhere in the interconnectedSystem class: the negative sign appears as one of the elements in summing junction's D matrix.

…of whether connection was explicit or implicit and issue a warning in connection_table if explicit

sawyerbfuller · 2023-08-14T20:05:22Z

@murrayrm I incorporated (most of) your suggested updates:

function warns if connections were made explicitly
- to do this, I added a field to InterconnectedSystem.connection_type that indicates how connections were made: 'explicit'-ly, 'implicit'-ly, or not at all (None)
table adds an elipsis if any column gets too long; user can now specify column width. Example:

    P1 = ct.ss(1,1,1,0, inputs='u', outputs='y', name='Plant1')
    P2 = ct.tf(10, [.1, 1], inputs='e', outputs='y', name='Plant2')
    P3 = ct.tf(10, [.1, 1], inputs='x', outputs='y', name='Plant3')
    L = ct.interconnect([P1, P2, P3], inputs=['e', 'u', 'x'], outputs='y')
    L.connection_table(show_names=True, column_width=23)

signal    | source               | destination
--------------------------------------------------------
x         | input                | Plant3               
e         | input                | Plant2               
u         | input                | Plant1               
y         | Plant1, Plant2, Pl.. | output

renamed function connection_table to make it more general, allowing for it to print a similar, but probably differently-formatted table for explicit connectiosn in the future
I don't know of an obvious way to indicate negative connections. perhaps a future PR?

murrayrm · 2023-08-19T17:53:02Z

control/nlsys.py

+
+        Parameters
+        ----------
+        show_names : bool (optional)


Numpydoc standard notation is bool, optional (comma instead of parens). I'm not sure if Sphinx will parse or not, but probably better to change so that it is consistent with other documentation.

murrayrm · 2023-08-19T17:53:16Z

control/nlsys.py

+            each system. Default is False because system name is not usually
+            specified when performing implicit interconnection using
+            :func:`interconnect`.
+        column_width : int (optional)


(optional) → , optional

murrayrm · 2023-08-19T17:54:35Z

control/nlsys.py

@@ -1955,7 +2032,7 @@ def interconnect(
        signals are given names, then the forms 'sys.sig' or ('sys', 'sig')
        are also recognized.  Finally, for multivariable systems the signal
        index can be given as a list, for example '(subsys_i, [inp_j1, ...,
-        inp_jn])'; as a slice, for example, 'sys.sig[i:j]'; or as a base
+        inp_jn])'; or as a slice, for example, 'sys.sig[i:j]'; or as a base


Wasn't this correct before? Now it says 'a; or b; or c' instead of 'a; b; or c'.

murrayrm · 2023-08-19T18:07:42Z

control/nlsys.py

+
+def connection_table(sys, show_names=False, column_width=32):
+    """Print table of connections inside an interconnected system model.
+
+    Intended primarily for :class:`InterconnectedSystems` that have been
+    connected implicitly using signal names.
+
+    Parameters
+    ----------
+    sys : :class:`InterconnectedSystem`
+        Interconnected system object
+    show_names : bool (optional)
+        Instead of printing out the system number, print out the name of
+        each system. Default is False because system name is not usually
+        specified when performing implicit interconnection using
+        :func:`interconnect`.
+    column_width : int (optional)
+        Character width of printed columns
+
+
+    Examples
+    --------
+    >>> P = ct.ss(1,1,1,0, inputs='u', outputs='y', name='P')
+    >>> C = ct.tf(10, [.1, 1], inputs='e', outputs='u', name='C')
+    >>> L = ct.interconnect([C, P], inputs='e', outputs='y')
+    >>> L.connection_table(show_names=True) # doctest: +SKIP
+    signal    | source                  | destination
+    --------------------------------------------------------------
+    e         | input                   | C
+    u         | C                       | P
+    y         | P                       | output
+    """
+    assert isinstance(sys, InterconnectedSystem), "system must be"\
+        "an InterconnectedSystem."
+
+    sys.connection_table(show_names=show_names, column_width=column_width)


Not something that we need to fix in this PR, but we should eventually adopt some mechanism for creating duplicate docstrings from a single primary docstring (see, for example, the discussion here).

murrayrm · 2023-08-19T18:08:48Z

control/statesp.py

@@ -1459,7 +1459,7 @@ class LinearICSystem(InterconnectedSystem, StateSpace):

    """

-    def __init__(self, io_sys, ss_sys=None):
+    def __init__(self, io_sys, ss_sys=None, connection_type=None):


Is there a reason to allow the connection type to be overridden? It seems like this should just be inherited from the underlying InterconnectedSystem.

sawyerbfuller added 6 commits July 21, 2023 14:24

fix name of default name for duplcated system in interconnect

aa9ef95

remove namedio from docstrings

5e92bb4

add signal_table method to show implicit interconnections

4e96553

unit test

b5925c3

add signal_table function, test against show_names

614de83

remove **kwargs

91aac8f

sawyerbfuller added 2 commits July 25, 2023 10:28

switch docstring example to use system names

796acad

add tests for auto-sum and auto-split

b787a9e

sawyerbfuller added 3 commits August 14, 2023 11:10

rename function to connection_table, add option to change column width

c2b4480

added field to interconnectedSystem and LinearICSystem to keep track …

58e1b60

…of whether connection was explicit or implicit and issue a warning in connection_table if explicit

added connection_table to documentation in control.rst

3637997

murrayrm reviewed Aug 19, 2023

View reviewed changes

change "(optional)" to ", optional" per numpydoc

5c23f1a

murrayrm merged commit 405bf77 into python-control:main Sep 16, 2023
14 checks passed

SoundsSerious mentioned this pull request Jan 21, 2024

Display Connection Matrix #963

Open

murrayrm added this to the 0.10.0 milestone Mar 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

print a connection table for interconnected systems #925

print a connection table for interconnected systems #925

sawyerbfuller commented Jul 24, 2023 •

edited

coveralls commented Jul 24, 2023 •

edited

murrayrm commented Jul 30, 2023

sawyerbfuller commented Aug 2, 2023 •

edited

sawyerbfuller commented Aug 14, 2023 •

edited

murrayrm Aug 19, 2023

murrayrm Aug 19, 2023

murrayrm Aug 19, 2023

murrayrm Aug 19, 2023

murrayrm Aug 19, 2023

print a connection table for interconnected systems #925

print a connection table for interconnected systems #925

Conversation

sawyerbfuller commented Jul 24, 2023 • edited

coveralls commented Jul 24, 2023 • edited

murrayrm commented Jul 30, 2023

sawyerbfuller commented Aug 2, 2023 • edited

sawyerbfuller commented Aug 14, 2023 • edited

murrayrm Aug 19, 2023

Choose a reason for hiding this comment

murrayrm Aug 19, 2023

Choose a reason for hiding this comment

murrayrm Aug 19, 2023

Choose a reason for hiding this comment

murrayrm Aug 19, 2023

Choose a reason for hiding this comment

murrayrm Aug 19, 2023

Choose a reason for hiding this comment

sawyerbfuller commented Jul 24, 2023 •

edited

coveralls commented Jul 24, 2023 •

edited

sawyerbfuller commented Aug 2, 2023 •

edited

sawyerbfuller commented Aug 14, 2023 •

edited