Updated factory functions to provide consistent functionality in terms of copying signal/system names, overriding system/signal names, and converting between classes.
Updated docstrings for all I/O classes and factory functions to provide consistent information and added unit tests to ensure consistency. The main changes were to make sure that all factory function parameters are documented while having the class docstrings focus on the primary parameters + attributes (with a reference to the factory function for more detailed documentation).
Updated thetf() factory function to allow a 2D list of SISO transfer functions to be given as a means of creating a MIMO transfer function (use the newcombine_tf function). This is redundant withcombine_tf, but seemed useful to include in the factory function for the special case of creating a MIMO transfer function via a set of SISO expressions. E.g.:
```
s = ct.tf('s')sys = ct.tf([[1/s, 0], [(s+1)/(s**2 + 2s + 1), 1/(s+1)]])
```
[8f3615b, 22 Dec '24] Added processing of theparams keyword tocreate_statefbk_iosystem.
[e4d373c, 22 Dec '24] Updatedinterconnect() to combine parameter lists of subsystems ifparams is not specified.
Changed some methods and attributes to be hidden so that they don't get flagged in docstring testing. These are functions that are not needed by the user (eg,FrequencyResponseData._ifunc, which is the internally generated interpolation function if thesmooth keyword is used).
Changed the internal representation of transfer function numerators and denominators to a 2D array of arrays instead of a 2D nested list of arrays. This allows user of NumPy routines when accessing input/output pairs (eg,sys.num_array[i, j] instead ofsys.num_list[i][j]). The nested list representation is still available via the{num,den}_list properties and thenum andden attributes retrieve the nested list structure for backward compatibility. (We may want to change that in the future, but it could break user code where someone is accessing the numerator and denominator polynomials directly.)
Add the ability fornlsys to create aNonlinearIOSystem representation of aStateSpace system. It's not clear that this is so useful for a user (sinceStateSpace is a subclass ofNonlinearIOSystem), but it is handy for testing since you can quickly create a nonlinear I/O system from a linear system but have it treated as a "pure"NonlinearIOSystem instead of aStateSpace system.
Got rid of redundant licensing information at the top of files that were modified (the information is contained in theLICENSE file in the main directory).

Note: this PR is likely to conflict with#1081, so the plan is to review/merge#1081 first, then rebase this PR on top of that one.

murrayrm added11 commits

December 20, 2024 07:58

add shape property to iosys

0003047

update FRD to copy signal/system names on sampling

6c852c3

convert internal representation of TF num/den to ndarray (vs list^2)

6df0919

add tf() support for array of SISO [combine_tf] + documentation updates

9f8ff40

change num/den and _num/_den to num/den_{list,array}

a7fd763

updated iosys class/factory function documentation + docstring unit t…

2c7ca9a

…esting

ss -> nlsys with name preservation/override + docstring update

1547326

update flatsys() documentation

319a8ea

add unit test to insure all factory functions handle renaming

2f2e372

document and fix {input,output,state}_prefix parameters

7a79342

small docstring and unit test warning cleanup

34d5d03

Copy link

coveralls commentedDec 20, 2024•
edited
Loading

coverage: 94.79% (+0.06%) from 94.73%
when pulling1061445 on murrayrm:iosys_enhance-07Dec2024
into9fa4aac on python-control:main.

slivingston self-requested a review

December 20, 2024 21:54

murrayrm added2 commits

December 22, 2024 09:50

add param processing to create_statefbk_iosystem

8f3615b

update default params computation in interconnect()

e4d373c

murrayrm force-pushed theiosys_enhance-07Dec2024 branch from8888c03 toe4d373cCompare

December 23, 2024 07:10

Copy link

Member

slivingston commentedDec 28, 2024

I will finish reviewing this tomorrow

slivingston reviewed

Dec 29, 2024

View reviewed changes

Copy link

Member

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 made several comments about misprints. Code appears to be correct. I am going to examine it a bit more in the morning.

control/tests/docstrings_test.py Outdated

		#
		# List of arguments described in class docstrings
		#
		# These are the minimal arguments needed to initialized the class. Optional

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

	# These are the minimal arguments needed toinitialized the class. Optional
	# These are the minimal arguments needed toinitialize the class. Optional

control/tests/docstrings_test.py Outdated

		# List of attributes described in class docstrings
		#
		# This is the list of attributes for the class that are not already listed
		# as parameters used to inialize the class. These should all be defined

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

	# as parameters used toinialize the class. These should all be defined
	# as parameters used toinitialize the class. These should all be defined

control/frdata.py Outdated

		frequency : 1D array
		Array of frequency points for which data are available.
		ninputs, noutputs : int
		Number of inputs and outputs signals.

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

	Numberofinputsandoutputssignals.
	Numberofinputandoutputsignals.

which is consistent with usage below (description ofinput_labels, output_labels), or could be "Number of inputs and outputs."

control/frdata.py Outdated

		magnitude : array
		Magnitude of the frequency response, indexed by frequency.
		phase : array
		Magnitude of the frequency response, indexed by frequency.

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

	Magnitudeofthefrequencyresponse,indexedbyfrequency.
	Phaseofthefrequencyresponse,indexedbyfrequency.

control/xferfcn.py Outdated


		Notes
		-----
		The attribues 'num' and 'den' are 2-D lists of arrays containing MIMO
		numerator and denominator coefficients. For example,
		The numerator and denominator polynomials are stored as 2D ndarray's

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

	Thenumeratoranddenominatorpolynomialsarestoredas2Dndarray's
	Thenumeratoranddenominatorpolynomialsarestoredas2Dndarrays

Style used in the NumPy documentation is "ndarrays" (no apostrophe); e.g., inhttps://numpy.org/doc/stable/user/basics.indexing.html andhttps://numpy.org/doc/stable/user/basics.creation.html.

control/statesp.py Outdated

		#
		# Author: Richard M. Murray
		# Date: 24 May 09
		# Revised: Kevin K. Chen, Dec 10

Copy link

Member

Choose a reason for hiding this comment

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

Outside the scope of this PR, but eventually we should consider removing comments with author names like this or add a note that

these dates are from before moving to Git for version control
and the up-to-date authorship is available in commit history.

Copy link

MemberAuthor

Choose a reason for hiding this comment

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

I took a cut at this in the commit that is coming next. See what you think and we can fix things up globally (I have another PR in the works where I could do that).

Copy link

Member

Choose a reason for hiding this comment

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

I like your proposed change: it is concise and shows a simple command to get the list of authors.

control/xferfcn.py Outdated

		to prettier LaTeX representation """
		"""Superscript all digits in a polynomial string and convert float
		coefficients in scientific notation to prettier LaTeX
		representation.

Copy link

Member

Choose a reason for hiding this comment

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

Docstring alignment is odd for this function.

update to address 29 Dec comments by@slivingston

e10d481

slivingston reviewed

Jan 4, 2025

View reviewed changes

control/iosys.py Outdated

Comment on lines 154 to 155

		repr_format : str
		String representation format ('iosys' or 'loadable').

Copy link

Member

Choose a reason for hiding this comment

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

Is there arepr_format attribute, or is this referring to__repr__? If the latter, I find it confusing to write "repr_format" in this list because it is not the name of an attribute. Also, note that__repr__ is astandard method described in the Python documentation, so probably OK to not document it here.

Copy link

MemberAuthor

murrayrmJan 4, 2025

Choose a reason for hiding this comment

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

I think this crept through from a separate PR that I am working on (and will show up next). Will delete from here (and one other spot where it shows up).

control/tests/xferfcn_test.py Outdated

Comment on lines 34 to 37

		# 13 Dec 2024: This now works correctly: creates static array (as tf)
		# with pytest.raises(TypeError):
		# TransferFunction([[0., 1.], [2., 3.]], [[5., 2.], [3., 0.]])
		# good input

Copy link

Member

slivingstonJan 4, 2025

Choose a reason for hiding this comment

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

I recommend deleting this comment block (from "13 Dec..." to "good input"). The fact that this was previously tested as a known error is already recorded in commit history. If the goal with this comment is to notify users about the behavior change, then probably it will be easier to find in the next release announcement.

Copy link

MemberAuthor

murrayrmJan 4, 2025

Choose a reason for hiding this comment

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

I'll remove this test completely, since it is now longer a "bad input type" (from the name of the test method).

control/xferfcn.py

Comment on lines +222 to 225

		forarr in[num,den]:
		for poly innp.nditer(arr, flags=['refs_ok']):
		if poly.item().size > 1:
		static = False

Copy link

Member

slivingstonJan 4, 2025

Choose a reason for hiding this comment

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

Suggested change

	forarrin [num,den]:
	forpolyinnp.nditer(arr,flags=['refs_ok']):
	ifpoly.item().size>1:
	static=False
	forarrin [num,den]:
	ifnotstatic:
	break
	forpolyinnp.nditer(arr,flags=['refs_ok']):
	ifpoly.item().size>1:
	static=False
	break

This is a small optimization: I noticed that afterstatic = False is reached, the outer and inner loops can be exited immediately. For example, the system in the PR description:

s = ct.tf('s')sys = ct.tf([[1/s, 0], [(s+1)/(s**2 + 2*s + 1), 1/(s+1)]])

results in several cases of needlessly reachingstatic = False more than once. However, I did not rigorously quantify the potential benefit, so OK to not do this.

Copy link

MemberAuthor

murrayrmJan 4, 2025

Choose a reason for hiding this comment

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

Good catch.

Copy link

MemberAuthor

murrayrmJan 4, 2025•
edited
Loading

Choose a reason for hiding this comment

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

Update coming in a bit to address these.

@slivingston Are you still reviewing, or is it OK to merge? (I have two more PR's coming that build on this one.)

Copy link

Member

slivingstonJan 4, 2025

Choose a reason for hiding this comment

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

@murrayrm Sorry for the delay. I am still reviewing but will be done in 30 minutes.

Copy link

Member

slivingstonJan 4, 2025

Choose a reason for hiding this comment

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

@murrayrm done now. There are 2 misprints and 1 bug. After handling these, ready to merge.

Copy link

Contributor

sawyerbfullerJan 4, 2025

Choose a reason for hiding this comment

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

Great PR. Just came here to note that there is already an_isstatic method that is also short-circuiting. Can we use the same code for both - perhaps by moving the code to a separate function that tests num and den, and a method that calls it?

Remove repr_format mentions; fix xferfcn static check per@slivingston

bf19193

slivingston reviewed

Jan 4, 2025

View reviewed changes

control/nlsys.py Outdated

Comment on lines 1336 to 1337

		lambda t, x, u, params: sys.A @ x + sys.B @ u,
		lambda t, x, u, params: sys.C @ x + sys.D @ u, **kwargs)

Copy link

Member

slivingstonJan 4, 2025

Choose a reason for hiding this comment

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

Suggested change

	lambdat,x,u,params:sys.A @x+sys.B @u,
	lambdat,x,u,params:sys.C @x+sys.D @u,**kwargs)
	lambdat,x,u,params:sys_ss.A @x+sys_ss.B @u,
	lambdat,x,u,params:sys_ss.C @x+sys_ss.D @u,**kwargs)

Copy link

Member

slivingstonJan 4, 2025

Choose a reason for hiding this comment

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

This is a bug and needs to be fixed before merging.

Copy link

MemberAuthor

murrayrmJan 4, 2025

Choose a reason for hiding this comment

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

Good catch on this. In the latest update, I added a unit test that catches the bug (and fixed the bug).

slivingston reviewed

Jan 4, 2025

View reviewed changes

control/statefbk.py Outdated

		@@ -751,6 +752,10 @@ def create_statefbk_iosystem(
		System name. If unspecified, a generic name <sys[id]> is generated
		with a unique integer id.

		params : dict, optional
		System parameter values. By default, these will be copied from
		`sys` and `ctrl`, but can be overriden with this keyword.

Copy link

Member

slivingstonJan 4, 2025

Choose a reason for hiding this comment

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

Suggested change

	`sys`and`ctrl`,butcanbeoverridenwiththiskeyword.
	`sys`and`ctrl`,butcanbeoverriddenwiththiskeyword.

control/statesp.py Outdated


		``ss(args, inputs=['u1', ..., 'up'], outputs=['y1', ..., 'yq'], states=['x1', ..., 'xn'])``
		``ss(*args, inputs=['u1', ..., 'up'], outputs=['y1', ..., 'yq'],
		states=['x1', ..., 'xn'])``

Copy link

Member

slivingstonJan 4, 2025

Choose a reason for hiding this comment

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

Splitting this in-line code expression across 2 lines breaks Sphinx processing, resulting in

Compare with the rendering ofss(A, B, C, D, dt) in the above image and the rendering of these in the current docs athttps://python-control.readthedocs.io/en/0.10.1/generated/control.ss.html

control/xferfcn.py

Comment on lines +222 to 225

		forarr in[num,den]:
		for poly innp.nditer(arr, flags=['refs_ok']):
		if poly.item().size > 1:
		static = False

Copy link

Member

slivingstonJan 4, 2025

Choose a reason for hiding this comment

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

@murrayrm Sorry for the delay. I am still reviewing but will be done in 30 minutes.

addressed@slivingstonreview comments

1061445

murrayrm merged commit93c4c8d intopython-control:main

Jan 4, 2025

22 checks passed

murrayrm mentioned this pull request

Jan 4, 2025

Remove redundant _isstatic calculation in xferfcn#1090

Closed

murrayrm added this to the0.10.2 milestone

Feb 19, 2025

Labels

None yet

4 participants

Movatterモバイル変換

I/O system enhancements#1082

I/O system enhancements#1082

Uh oh!

Conversation

murrayrm commentedDec 20, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

coveralls commentedDec 20, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

slivingston commentedDec 28, 2024

Uh oh!

slivingston left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

murrayrmJan 4, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

murrayrm commentedDec 20, 2024•
edited
Loading

coveralls commentedDec 20, 2024•
edited
Loading

murrayrmJan 4, 2025•
edited
Loading