Skip to content

Proposal for multiplying a NumPy array by a scalar TransferFunction #1076

Closed
#1081
Closed
Proposal for multiplying a NumPy array by a scalar TransferFunction#1076
#1081

Description

@sdahdah
sdahdah
opened on Dec 5, 2024

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
    murrayrm
    on Dec 6, 2024
    Member

    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).

  2. sdahdah commented on Dec 6, 2024

    @sdahdah
    sdahdah
    on Dec 6, 2024
    ContributorAuthor

    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
    murrayrm
    on Dec 6, 2024
    Member

    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).

  4. murrayrm commented on Dec 7, 2024

    @murrayrm
    murrayrm
    on Dec 7, 2024
    Member

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

  5. sdahdah commented on Dec 12, 2024

    @sdahdah
    sdahdah
    on Dec 12, 2024
    ContributorAuthor

    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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions