update model_reduction to allow input/output selection + unstable warning (vs exception) by murrayrm · Pull Request #1074 · python-control/python-control

murrayrm · 2024-12-04T20:28:51Z

This PR updates the model_reduction function to add some additional functionality. From the docstring:

This function produces a reduced-order model of a system by eliminating specified inputs, outputs, and/or states from the original system. The specific states, inputs, or outputs that are eliminated can be specified by either listing the states, inputs, or outputs to be eliminated or those to be kept.

Specific updates:

Changed the ELIM positional parameter to elim_states (positional or keyword).
Added keep_states to allow specifying the list of states to keep instead of those to eliminate.
Added elim_inputs, elim_outputs, keep_inputs, keep_outputs to allow selection of inputs and outputs.
Specifications of states, inputs, and outputs can be done by a list of integers (original implementation) as well as slices or lists of signal names (new).
In the case of eliminating states, inputs, or outputs from an unstable system, a warning is issued (versus an exception) and this warning can be suppressed using warn_unstable=False.
Removed the copyright info from the top of modelsimp.py since we know just use the LICENSE file.
Add unit tests and documentation updates.
Cleaned up import order (isort -m2) and removed extraneous spaces.

This PR was motivated by an example from FBS2e (see steering.py), where I wanted to extract the dynamics of a subsystem (and the original system had an eigenvalue at the origin). It is possible to accomplish the same result by using the old implementation (assuming the exception is changed to a warning) and then indexing the system directly to handle the inputs and outputs, but it seemed natural to include in a single function call.

coveralls · 2024-12-04T20:45:29Z

coverage: 94.756% (+0.07%) from 94.689%
when pulling eff6c40 on murrayrm:modred_unstable-28Nov2024
into 69efbbe on python-control:main.

slivingston

Good improvement! I made several stylistic and misprint-related comments. Else, ready to merge.

slivingston · 2024-12-05T04:53:23Z

control/modelsimp.py

+    eliminated or those to be kept.
+
+    Two methods of state reduction are possible: 'truncate' removes the
+    states marked for elimination, while 'matchdc' replaces the the


Suggested change

states marked for elimination, while 'matchdc' replaces the the

states marked for elimination, while 'matchdc' replaces the

slivingston · 2024-12-05T04:56:17Z

control/modelsimp.py

+    unstable eigenvalues, since in those situations the stability reduced
+    order model may be different that the stability of the full model.  No
+    other checking is done, so users to be careful not to render a system


Suggested change

unstable eigenvalues, since in those situations the stability reduced

order model may be different that the stability of the full model. No

other checking is done, so users to be careful not to render a system

unstable eigenvalues, since in those situations the stability of the reduced

order model may be different than the stability of the full model. No

other checking is done, so users must be careful not to render a system

slivingston · 2024-12-05T04:56:49Z

control/modelsimp.py

-        dico = 'C'
-    else:
-        raise NotImplementedError("Function not implemented in discrete time")
+    States, inputs, and outputs can be specified using integer offers or


Suggested change

States, inputs, and outputs can be specified using integer offers or

States, inputs, and outputs can be specified using integer offsets or

slivingston · 2024-12-05T18:31:25Z

control/tests/modelsimp_test.py

+    ({'elim_inputs': [0, 1, 2]}, 5, 3, 0),              # no inputs
+    ({'elim_outputs': [0, 1, 2]}, 5, 0, 3),             # no outputs
+    ({'elim_states': [0, 1, 2, 3, 4]}, 0, 3, 3),        # no states
+    ({'elim_states': [0, 1], 'keep_states': [1, 2]}, None, None, None)


Suggested change

({'elim_states': [0, 1], 'keep_states': [1, 2]}, None, None, None)

({'elim_states': [0, 1], 'keep_states': [1, 2]}, None, None, None),

Trailing comma because, then, adding items in the future will not cause a diff on this line, as recommended by PEP 8 and enforced by Black.

slivingston · 2024-12-05T19:04:16Z

control/modelsimp.py

+    See Also
+    --------
+    balanced_reduction : Eliminate states using Hankel singular values.
+    minimal_realization : Eliminate unreachable or unobseravble states.


Suggested change

minimal_realization : Eliminate unreachable or unobseravble states.

minimal_realization : Eliminate unreachable or unobservable states.

slivingston · 2024-12-05T19:06:08Z

control/modelsimp.py

+            warnings.warn("System is unstable; reduction may be meaningless")
+
+    # Utility function to process keep/elim keywords
+    def _process_elim_or_keep(elim, keep, labels, allow_both=False):


Any reason to keep allow_both? _process_elim_or_keep() is a function defined only in the scope of model_reduction(), and all calls to it have the default allow_both value (allow_both=False).

slivingston · 2024-12-05T19:09:04Z

control/modelsimp.py

    method : string
-        Method of removing states in `ELIM`: either 'truncate' or
-        'matchdc'.
+        Method of removing states in `elim`: either 'truncate' or


Suggested change

Method of removing states in `elim`: either 'truncate' or

Method of removing inputs, outputs, and/or states: either 'truncate' or

This is confusing because there is no elim parameter and because inputs and outputs can be removed as well, in addition to states.

Will update, but this only applies to the way states are removed.

slivingston · 2024-12-05T19:15:09Z

control/modelsimp.py

+
+    """
+    if not isinstance(sys, StateSpace):
+        raise TypeError("system must be a a StateSpace system")


Suggested change

raise TypeError("system must be a a StateSpace system")

raise TypeError("system must be a StateSpace system")

murrayrm added 7 commits November 29, 2024 08:23

allow state elimination on unstable systems with warning

aad5502

update unit tests

e6824d7

initial refactoring of model_reduction (no changes in functionality)

77b25d5

add input/output removal + unit tests

ae43212

update documentation for basic functionality

310798e

add slice processing

b96a039

add signal name processing

412ef97

slivingston self-requested a review December 5, 2024 17:06

slivingston reviewed Dec 5, 2024

View reviewed changes

updates to address @slivingston review comments

eff6c40

slivingston approved these changes Dec 7, 2024

View reviewed changes

murrayrm merged commit e2c0ff8 into python-control:main Dec 7, 2024

murrayrm added this to the 0.10.2 milestone Feb 19, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

update model_reduction to allow input/output selection + unstable warning (vs exception)#1074

update model_reduction to allow input/output selection + unstable warning (vs exception)#1074
murrayrm merged 8 commits intopython-control:mainfrom
murrayrm:modred_unstable-28Nov2024

murrayrm commented Dec 4, 2024

Uh oh!

coveralls commented Dec 4, 2024 •

edited

Loading

Uh oh!

slivingston left a comment

Uh oh!

slivingston Dec 5, 2024

Uh oh!

slivingston Dec 5, 2024

Uh oh!

slivingston Dec 5, 2024

Uh oh!

slivingston Dec 5, 2024

Uh oh!

slivingston Dec 5, 2024

Uh oh!

slivingston Dec 5, 2024

Uh oh!

slivingston Dec 5, 2024

Uh oh!

murrayrm Dec 7, 2024

Uh oh!

slivingston Dec 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	states marked for elimination, while 'matchdc' replaces the the
	states marked for elimination, while 'matchdc' replaces the

	States, inputs, and outputs can be specified using integer offers or
	States, inputs, and outputs can be specified using integer offsets or

	({'elim_states': [0, 1], 'keep_states': [1, 2]}, None, None, None)
	({'elim_states': [0, 1], 'keep_states': [1, 2]}, None, None, None),

	minimal_realization : Eliminate unreachable or unobseravble states.
	minimal_realization : Eliminate unreachable or unobservable states.

	Method of removing states in `elim`: either 'truncate' or
	Method of removing inputs, outputs, and/or states: either 'truncate' or

	raise TypeError("system must be a a StateSpace system")
	raise TypeError("system must be a StateSpace system")

Comments

Conversation

murrayrm commented Dec 4, 2024

Uh oh!

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

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coveralls commented Dec 4, 2024 •

edited

Loading