GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add `combine()` and `split()` functions for transfer matrices by sdahdah · Pull Request #1073 · python-control/python-control

Conversation

@sdahdah

@sdahdah sdahdah commented Dec 3, 2024

Copy link
Copy Markdown
Contributor

RE #325

I've run into a few times where it would be convenient to construct MIMO TransferMatrix objects from SISO ones, or the reverse. In particular, this came up as I've been working on a D-K iteration package, dkpy. The specific implementation here is from utilities.py in dkpy.

I saw there was a related issue (#325 ) and people seemed open to it. Specifically, I've added

  • control.combine() that takes an ArrayLike of TransferFunction objects, scalars, and NumPy arrays, and creates a MIMO transfer matrix, and
  • control.split() that takes a MIMO TransferFunction and returns a NumPy array with SISO TransferFunction objectss.

I'm new to the python-control codebase, so I'm not 100% sure if I've placed things in the correct files, but I've added unit tests and would appreciate some feedback.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
sdahdah added 2 commits December 3, 2024 16:43
@sdahdah
Add combine and split functions
62d6f83
@sdahdah
Remove pass typo
8ac3adc
@coveralls

coveralls commented Dec 3, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Coverage Status

coverage: 94.723% (+0.03%) from 94.689%
when pulling 1cc84a7 on sdahdah:main
into 69efbbe on python-control:main.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

murrayrm
murrayrm requested changes Dec 4, 2024
View reviewed changes

@murrayrm murrayrm left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks for the contribution! This will make a nice addition.

Lots of little suggestions herein, mainly to keep things consistent with other code in the package.

One bigger picture question is on the function names: since these only work for transfer functions, I wonder if the function names shouldn't be combine_tf and split_tf (using 'tf' rather than transfer_function since the factor function is tf).

Also, I think there needs to be some checking for size consistency. For example, consider the following code:

import control as ct
sys1 = ct.tf(ct.rss(2, 1, 2))
sys2 = ct.tf(ct.rss(2, 2, 1))
sys = ct.combine([[sys1, sys2]])

The first system has 1 output and the second system has two outputs, so I think this should generate an error. Instead, I get a 3 input (correct) by 1 output (incorrect) system. If instead I do

sys = ct.combine([[sys2, sys1]])

I get an error message:

    587 for col in row:
    588     for j_in in range(col.ninputs):
--> 589         num_row.append(col.num[j_out][j_in])
    590         den_row.append(col.den[j_out][j_in])
    591 num.append(num_row)

IndexError: list index out of range

Shouldn't both forms generate error messages (perhaps saying that there are an inconsistent number of outputs in row 1)?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread control/bdalg.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread control/bdalg.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread control/bdalg.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread control/bdalg.py
try:
for row in tf_array:
for tfn in row:
dt_list.append(getattr(tfn, "dt", None))

@murrayrm murrayrm Dec 4, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

dt will always exist for an InputOutputSystem, so this can just be tfn.dt.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@sdahdah sdahdah Dec 4, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The combine_tf() function allows NumPy arrays to be input as well, so you can do something like

control.combine_tf([
    [np.eye(2), np.zeros((2, 1))],
    [np.zeros((1, 2)), control.TransferFunction([1], [1, 0])],
])

So in that case, the NumPy arrays do not have a dt attribute. The dt will be taken from the TransferFunction object in this case.

I've found this useful when creating weighting functions for optimal control problems.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread control/bdalg.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread control/bdalg.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread control/bdalg.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread control/bdalg.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread control/bdalg.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

sdahdah added 5 commits December 4, 2024 11:11
@sdahdah
Add test and fix for mismatched row outputs
758fa9d
@sdahdah
Update documentation for exception
120984f
@sdahdah
Add input dimension check
76d4ee7
@sdahdah
Rename split and combine to split_tf and combine_tf
9f4bf4f
@sdahdah
Adjust naming and docstring conventions
dc36574
@sdahdah

sdahdah commented Dec 4, 2024

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback. Adding _tf to the function names makes sense to me.

I've also fixed the dimension checking and added a couple unit tests for that case I missed.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sdahdah added 5 commits December 4, 2024 12:03
@sdahdah
Add example with NumPy arrays
1cbd6b3
@sdahdah
Fix line length
68d52d8
@sdahdah
Fix line length in test file
35fa5b3
@sdahdah
Remove extra markup
8bc9487
@sdahdah
Add extra test case for wrong number of inputs
1cc84a7
@sdahdah
sdahdah requested a review from murrayrm December 6, 2024 21:03
murrayrm
murrayrm approved these changes Dec 7, 2024
View reviewed changes
@murrayrm
murrayrm merged commit da55654 into python-control:main Dec 7, 2024
@murrayrm murrayrm added this to the 0.10.2 milestone Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@murrayrm murrayrm murrayrm approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.10.2

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@sdahdah @coveralls @murrayrm
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.