Description

sdahdah

opened on Dec 5, 2024

Issue body actions

Currently, multiplying a SISO TransferFunction object by a NumPy array results in a dimension error:

import control
import numpy as np

tf = control.TransferFunction([1], [1, 0])
arr = np.array([
    [1, 0],
    [2, 1],
])

print(tf * arr)
print(arr * tf)

ValueError: C = A * B: A has 1 column(s) (input(s)), but B has 2 row(s) (output(s)).

I think it would be convenient if output was a MIMO TransferFunction object of the array's dimension. So, for example,

G = control.TransferFunction(
    [
        [[1], [0]],
        [[2], [1]],
    ],
    [
        [[1, 0], [1, 0]],
        [[1, 0], [1, 0]],
    ],
)

I would be happy to implement this and do a PR, but I want to make sure it's something the maintainers actually want. I would think all SISO systems (state-space, etc) should behave in the same way.

This appears to have been discussed in #459 , but I'm not sure what the conclusion was.

Activity

1. murrayrm commented on Dec 6, 2024

   

   murrayrm

   on Dec 6, 2024

   Member

   More actions

   I think it makes sense that if a "scalar" I/O system (= SISO system) is used in an operation involving either an ndarray or a MIMO transfer function, it should broadcast to the right shape.

   Specific examples/comments:

   • <2x2 MIMO system> * <1x1 SISO system> should broadcast the dimensions so that each element in the 2x2 system is multiplied by the SISO system. Same thing would happen with + or - (each element has the SISO system added or subtracted) and everything should also work if the arguments are switched.
   • * <1x1 SISO system> should generate an with each entry being the SISO system.
   • * becomes a MIMO system (as currently happens). I bring this one up because this is not what happens with ndarrays (you need to use @ to get matrix multiplication) => maybe we want to implement __matmul__?
   • If we do implement __matmul__, then I think <2x2 MIMO system> @ <1x1 SISO system> should probably generate an error, as it does with ndarrays of those sizes.

   For now, perhaps we just implement the SISO → MIMO broadcast functionality and leave out _matmul__? Note that all of this should work for all I/O systems, not just transfer functions. That might be tricky for NonlinearIOSystem's, so perhaps we just implement for LTI systems (FrequencyResponseData, StateSpace, TransferFuncion).

   Reacted by Steven Dahdah
2. sdahdah commented on Dec 6, 2024

   

   sdahdah

   on Dec 6, 2024

   ContributorAuthor

   More actions

   I think it makes sense that if a "scalar" I/O system (= SISO system) is used in an operation involving either an ndarray or a MIMO transfer function, it should broadcast to the right shape.

   Specific examples/comments:

   * <2x2 MIMO system> * <1x1 SISO system> should broadcast the dimensions so that each element in the 2x2 system is multiplied by the SISO system.  Same thing would happen with + or - (each element has the SISO system added or subtracted) and everything should also work if the arguments are switched.
   

   ^ I very much agree with this

   * * <1x1 SISO system> should generate an  with each entry being the SISO system.
   
   * *  becomes a  MIMO system (as currently happens).  I bring this one up because this is _not_ what happens with ndarrays (you need to use `@` to get matrix multiplication) => maybe we want to implement `__matmul__`?
   

   ^ I'm not sure I follow these two points

   * If we do implement `__matmul__`, then I think <2x2 MIMO system> @ <1x1 SISO system> should probably generate an error, as it does with ndarrays of those sizes.
   

   ^ I do think that this should raise an error if __matmul__ is implemented. My gut feeling is that implementing __matmul__ would be confusing though, since series() is essentially "multiplying transfer matrices." Is the idea that * would allow broadcasting and @ would not, but otherwise they would function identically?

   For now, perhaps we just implement the SISO → MIMO broadcast functionality and leave out _matmul__? Note that all of this should work for all I/O systems, not just transfer functions. That might be tricky for NonlinearIOSystem's, so perhaps we just implement for LTI systems (FrequencyResponseData, StateSpace, TransferFuncion).

   ^ I will take a look at this next week and see if I can find a nice way to do it for all I/O systems. If not, I will start small with just LTI.

   Should I also look at #1075 at the same time?
3. murrayrm commented on Dec 6, 2024

   

   murrayrm

   on Dec 6, 2024

   Member

   More actions

   Should I also look at #1075 at the same time?

   I'll probably take a look at this over the week and if the fix is simple, will submit a PR that you can build on. But if I haven't gotten to it by the time you start, it would be great to take care of #1075 at the same time (it will be in the same block of code).

   Reacted by Steven Dahdah
4. murrayrm commented on Dec 7, 2024

   

   murrayrm

   on Dec 7, 2024

   Member

   More actions

   #1075 is fixed. Make sure to build on PR #1078 when you make the changes for this issue.

   Reacted by Steven Dahdah
5. sdahdah commented on Dec 12, 2024

   

   sdahdah

   on Dec 12, 2024

   ContributorAuthor

   More actions

   Thanks. I started working on it earlier this week, and I will probably be ready to do a PR next week.
6. murrayrm

   closed this as completedin #1081on Jan 20, 2025

Issue actions