Small improvements to nlsys, bdalg by murrayrm · Pull Request #1019 · python-control/python-control

murrayrm · 2024-06-30T15:14:24Z

This PR provides some small changes to nonlinear I/O systems process:

Allows system name to be overridden in linearize, even if copy_names is False.
Allows renaming of system/signal names in bdalg functions:
```
sys = ct.series(sys1, sys2, inputs='u', outputs='y')
```
New update_names method for I/O systems that allows signal and system names to be updated.
Refactoring of code for processing x0, u0 keywords in linearize and input_output_response to provide common functionality in allowing concatenation of lists and zero padding ("vector element processing").
Improved error messages when x0 and u0 don't match the expected size.
If no output function is given in nlsys, which provides full state output, the output signal names are set to match the state names.
Updated unit tests and documentation (including a new section on "vector element processing").

…g fix

coveralls · 2024-06-30T15:26:18Z

coverage: 94.603% (+0.09%) from 94.518%
when pulling a402a7f on murrayrm:nlsys_improvements-24May2024
into feeb56a on python-control:main.

roryyorke

Functionality looks good. Minor suggestions and remarks.

roryyorke · 2024-07-07T14:47:26Z

control/bdalg.py



-def series(sys1, *sysn):
+def series(sys1, *sysn, **kwargs):


kwargs not documented in docstring, here or the other functions in bdalg.py.

roryyorke · 2024-07-07T14:50:16Z

control/iosys.py

+
+        Parameters
+        ----------
+        inputs : int, list of str, or None


name kwarg not documented

roryyorke · 2024-07-07T14:52:01Z

control/iosys.py

@@ -825,6 +865,7 @@ def _process_labels(labels, name, default):
 #
 import re


this can go - imported at top (not changed in this PR, tho...)

roryyorke · 2024-07-07T14:55:02Z

control/iosys.py

+
+        Parameters
+        ----------
+        inputs : int, list of str, or None


explanation (or pointer to such - think this convention used in a few places?) of what integer and None do.

Possibly out-of-scope: what about a dict, of {oldname1:newname1, oldname2:newname2,...} - in this case don't have to specify all inputs (or outputs, or states...)

roryyorke · 2024-07-07T14:57:26Z

control/iosys.py

+
+        """
+        self.name = kwargs.pop('name', self.name)
+        if kwargs.get('inputs', None):


Suggested change

if kwargs.get('inputs', None):

if 'inputs' in kwargs:

Easier to read?

dict.get has signature get(key, default=None), so if you want to use get you don't need None.

roryyorke · 2024-07-07T15:06:28Z

control/nlsys.py

+            v = np.array(v).reshape(-1)             # convert to 1D array
+            val_list += v.tolist()                  # add elements to list
+        val = np.array(val_list)
+    elif np.isscalar(arg) and size is not None:     # extend scalars


does this funciton need to handle the case when np.isscalar(arg) and size is None, e.g., do val = np.array([arg]) ?

Added this case.

slivingston · 2024-07-09T06:01:44Z

control/nlsys.py

+    # Process input argument
+    #
+    # The input argument is interpreted very flexibly, allowing the
+    # use of listsa and/or tuples of mixed scalar and vector elements.


Suggested change

# use of listsa and/or tuples of mixed scalar and vector elements.

# use of lists and/or tuples of mixed scalar and vector elements.

coveralls · 2024-07-09T06:04:53Z

coverage: 94.628% (+0.1%) from 94.518%
when pulling 8e123aa on murrayrm:nlsys_improvements-24May2024
into feeb56a on python-control:main.

slivingston · 2024-07-09T06:12:27Z

doc/iosys.rst

+* Vector elements are zero padded to the required length. If you
+  specify only a portion of the values for states or inputs, the
+  remaining values are taken as zero.  (If the final element in the
+  given vector is non-zero, a warning is issues.)


Suggested change

given vector is non-zero, a warning is issues.)

given vector is non-zero, a warning is issued.)

slivingston · 2024-07-09T06:15:11Z

doc/iosys.rst

+state and input vectors, respectively.
+
+If we want to linearize the closed loop system around a process state
+``x0`` (with two elemenst) and an estimator state ``0`` (for both states),


Suggested change

``x0`` (with two elemenst) and an estimator state ``0`` (for both states),

``x0`` (with two elements) and an estimator state ``0`` (for both states),

slivingston · 2024-07-09T06:17:27Z

doc/iosys.rst

+``x0`` (with two elemenst) and an estimator state ``0`` (for both states),
+we can use the list processing feature::
+
+  H = clsys.liniearize([x0, 0], 0)


Suggested change

H = clsys.liniearize([x0, 0], 0)

H = clsys.linearize([x0, 0], 0)

slivingston · 2024-07-09T06:21:33Z

doc/iosys.rst

+second argument in the list ``[x0, 0]`` is a scalar and so the vector
+``[x0, 0]`` only has three elements instead of the required four.
+
+To run an input/output simulation with a sinsoidal signal for the first


Suggested change

To run an input/output simulation with a sinsoidal signal for the first

To run an input/output simulation with a sinusoidal signal for the first

murrayrm added 9 commits June 29, 2024 14:12

use state names as output names when outfcn=None

2b2de2b

bugfix: allow sysname override in linearize if copy_names is False

84a1e4d

remove redundant code

c568c59

improve error messages for inconsistent size info in nlsys + small bu…

36263d8

…g fix

refactor processing of x0, u0 keywords in nlsys

1f5de6f

_update process_vector_argument to always return 1D ndarray

b10e3e2

add documentation for vector element processing

0fab739

allow renaming of system/signal names in bdalg functions

8b85747

update test_update_names to not require slycot

a402a7f

murrayrm added this to the 0.10.1 milestone Jun 30, 2024

roryyorke reviewed Jul 7, 2024

View reviewed changes

address @roryyorke review comments

88e2945

slivingston reviewed Jul 9, 2024

View reviewed changes

fix typos pointed out by @slivingston

8e123aa

murrayrm merged commit 6406868 into python-control:main Jul 9, 2024

murrayrm deleted the nlsys_improvements-24May2024 branch July 9, 2024 14:39

murrayrm mentioned this pull request Jul 27, 2024

Version 0.10.1 release plans #1020

Closed

11 tasks

		@@ -825,6 +865,7 @@ def _process_labels(labels, name, default):
		#
		import re

	# use of listsa and/or tuples of mixed scalar and vector elements.
	# use of lists and/or tuples of mixed scalar and vector elements.

	given vector is non-zero, a warning is issues.)
	given vector is non-zero, a warning is issued.)

	``x0`` (with two elemenst) and an estimator state ``0`` (for both states),
	``x0`` (with two elements) and an estimator state ``0`` (for both states),

	H = clsys.liniearize([x0, 0], 0)
	H = clsys.linearize([x0, 0], 0)

	To run an input/output simulation with a sinsoidal signal for the first
	To run an input/output simulation with a sinusoidal signal for the first

Conversation

murrayrm commented Jun 30, 2024

Uh oh!

coveralls commented Jun 30, 2024

Uh oh!

roryyorke 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!

coveralls commented Jul 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

coveralls commented Jul 9, 2024 •

edited

Loading