Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
murrayrm merged 21 commits intopython-control:mainfrommurrayrm:iosys_repr-07Dec2024
Jan 13, 2025

Conversation

murrayrm
Copy link
Member

@murrayrmmurrayrm commentedJan 4, 2025
edited
Loading

This PR provides updates to the string representations of I/O systems, including both the__repr__ and__str__ functions (used forrepr(),str(), andprint()) as well as LaTeX output (via_repr_html_):

  • Created a set of standard_repr_* formats forInputOutputSystem'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 inLTI systems to show the state space matrices, numerator/denominator, or freuquency response data. This format is similar to what was done before this PR forLTI 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 functioniosys_repr (similar tonp.array_repr) that allows different representations of I/O systems via aformat parameter ('info', 'eval', or 'latex').
  • The transfer functiondisplay_format parameter is now handled in a dynamic fashion. If it is set toNone (default) on system creation, the display style will followconfig.defaults['xferfcn.display_format']. If set topoly orzpk on system creation, that overrides the default.
  • Updated thestr() (print) representation ofInputOutputSystems to includedt after the state/input/output names (suppressed if it matches the defaultdt).
  • Updated thestr() (print) representation forNonlinearIOSystems to include list of parameter labels.
  • Updated thestr() (print) representation forInterconnectedSystems to include the list of subsystems (in 'info' form) as well summaries of the connections and outputs matrix (might be an eventual replacement forconnection_table).
  • All representations now applynp.printoptions to system arrays before generating strings, so you get consistent formatting across representations (including latex) and you can use things likesuppress to display near-zero numbers as '0'.
  • Fixed up various line spacings instr() (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).
  • Addedexamples/repr_gallery.ipynb andexampes/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).
  • Addedconfig.defaults options 'iosys.repr_format' and 'iosys.repr_show_count' to control representation formats.
  • Thecombine_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 ofconfig.defaults to be called as a context manager, so that you can temporarily change default parameters (this is used inexamples/repr_gallery.py). (This is more general than this PR, but I used it extensively inrepr_gallery).
  • Updated docstrings and unit tests.

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

Some highlights (pulled fromrepr_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_dssInputs (1): ['u[0]']Outputs (1): ['y[0]']States (2): ['x[0]', 'x[1]']dt = 0.1A = [[ 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_nlInputs (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_icInputs (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 commentedJan 4, 2025
edited
Loading

Coverage Status

coverage: 94.686% (-0.1%) from 94.781%
when pulling0f0fad0 on murrayrm:iosys_repr-07Dec2024
intoa1791c9 on python-control:main.

Copy link
Contributor

@sawyerbfullersawyerbfuller 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 systemsu0 ,u1 vsu[0] ,u[1] for other systems a bug or a feature? Maybe a bug that this PR uncovered.

@murrayrm
Copy link
MemberAuthor

Is the different format for the inputs for stateless systemsu0 ,u1 vsu[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).

sawyerbfuller reacted with thumbs up emoji

@slivingstonslivingston self-requested a reviewJanuary 5, 2025 03:58
Copy link
MemberAuthor

@murrayrmmurrayrm 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
MemberAuthor

Choose a reason for hiding this comment

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

Current release also useseval as default, so this may not be needed. We could setiosys.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
MemberAuthor

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
MemberAuthor

@murrayrmmurrayrmJan 8, 2025
edited
Loading

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.


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

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.

@@ -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
MemberAuthor

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
MemberAuthor

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
MemberAuthor

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
MemberAuthor

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)

slivingston reacted with thumbs up emoji
Copy link
Member

@slivingstonslivingston 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.

murrayrm reacted with thumbs up emoji
# 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 someinterest
# changes in representation formats, but also has someinteresting

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
assertre.search(expected,out)!=None
assertre.search(expected,out)isnotNone

Preferred style (inPEP 8) isis 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
assertgetattr(sys,attr)==getattr(sys,attr)
assertgetattr(sys,attr)==getattr(new,attr)

@murrayrm
Copy link
MemberAuthor

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

@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.

@murrayrmmurrayrm merged commitec7dc8a intopython-control:mainJan 13, 2025
23 checks passed
@murrayrmmurrayrm mentioned this pull requestJan 13, 2025
6 tasks
@murrayrmmurrayrm added this to the0.10.2 milestoneFeb 19, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@slivingstonslivingstonslivingston approved these changes

@sawyerbfullersawyerbfullersawyerbfuller approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.10.2
Development

Successfully merging this pull request may close these issues.

4 participants
@murrayrm@coveralls@slivingston@sawyerbfuller

[8]ページ先頭

©2009-2025 Movatter.jp