Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multivariable interconnect functionality #881

Merged

Conversation

murrayrm
Copy link
Member

@murrayrm murrayrm commented Apr 2, 2023

This PR expands the interconnect function to allow a variety of "multivariable" specifications for connections, inputs, and outputs when systems have variables with names of the form sig[i]:

  • Specifying a signal as sys.sig will match all signals of the form sys.sig[i]. So a MIMO feedback system interconnection assuming the usual input/output naming conventions can be written as connections=[['P.u', 'C.y'], ['C.u', '-P.y']], inplist='C.u', outlist='P.y' (before this PR, you would have to write connections=[['P.u[0]', 'C.u[0]'], ['P.u[1]', 'C.u[1]'], ...]).
  • Signal ranges can be described using string specifications of the form sys.sig[i:j] or tuple specifications of the form (sys_index, [i1, ..., iN], gain) instead of having to write all signals out explicitly.
  • Signals can also be specified just using the base name, in which case all signals with that base name, regardless of the subsystem, will be used as part of a connection (typically used for input and output connections).
  • Specifying just the system name is interpreted as using all inputs or outputs (as appropriate in context). So a feedback system interconnections can be written using connections=[['P', 'C'], ['C', -P]], inplist='C', outlist='P' (note that the input to C will be the sum of the input to the new system and the negation of the plant output, so the error between the reference and the output).
  • New functions are available to find lists of inputs, outputs, and states from a (multivariable) description: sys.find_inputs, sys.find_outputs, sys.find_states. Each returns a list of indices (versus the singular version, which returns a single index).
  • Indexing of (linear) systems, such as linsys[2, 1], now preserves signal names and modifies the system name using configurable a prefix and suffix (default is to append '$indexed' to the system name).
  • Unit tests and documentation (including examples of calling formats) have been added.

Note: one change in this PR is that you now must use lists rather than tuples when specifying lists of signals. This is consistent with the documentation from previous versions, but some examples used tuples instead of lists. In order to handle the multi-variable case, the only place that a tuple should be used is as a low level signal specification. Everything else should be a list.

@coveralls
Copy link

coveralls commented Apr 2, 2023

Coverage Status

coverage: 94.607% (-0.02%) from 94.628% when pulling 55e2b55 on murrayrm:multivariable_interconnect-30Mar2023 into d834f7a on python-control:main.

@sawyerbfuller
Copy link
Contributor

Nice PR for this functionality. A few thoughts/comments:

  1. What is the rationale for requiring lists rather than tuples to send sequences of signal names? Is it that tuples in this context are interpreted differently? Not possible to just call list() on the input arguments? (one less bug to get stuck on?)
  2. Relatedly, when specifying signal ranges not as a slice, does this support the a somewhat hidden feature of numpy that indexing by tuples means something different than indexing by lists? Tuples are interpreted as coordinates into the array whereas lists are interpreted as selecting elements of the array. For example:
a = np.array((1,2,3))
a[0] // gives 1
a[(0,1]) // tuple indexing gives IndexError: too many indices for array: array is 1-dimensional, but 2 were indexed
a[[0,1]) // list indexing gives array([1, 2])

This could be used to access a subset of signals in a given name: sys.sig[[list_of_indices]]

  1. Does this work for name-based auto-connection, that is, no connections keyword, provided all signals of the same name have the same size?

@murrayrm
Copy link
Member Author

murrayrm commented Apr 8, 2023

  1. What is the rationale for requiring lists rather than tuples to send sequences of signal names? Is it that tuples in this context are interpreted differently? Not possible to just call list() on the input arguments? (one less bug to get stuck on?)

The problem is that with the new "abbreviated" ways of specifying signals, there can be ambiguity if you use a list instead of a tuple. For example, if we allowed tuples and lists to be used interchangeably, the output specification

outlist = ('P', 'z')

becomes ambiguous. Is this a single output with system 'P' and signal 'z' or did you mean the outputs of the system 'P' and also the signals 'z'? In this PR, this specification gets interpreted as the former case, whereas if you want the latter you would write

outlist=['P', 'z']

Note that here we have used the shorthand that you can specify the outputs of a block just by giving the name of the block ('P') and you can specify any output signal in the diagram (regardless of what subsystem it belongs to) just be specifying the signal name ('z'). (It is also possible that you mean signals 'P' and systems 'z', of course, and if there is ambiguity based on the specific subsystems then you get an error.)

  1. Relatedly, when specifying signal ranges not as a slice, does this support the a somewhat hidden feature of numpy that indexing by tuples means something different than indexing by lists? Tuples are interpreted as coordinates into the array whereas lists are interpreted as selecting elements of the array. ....

This could be used to access a subset of signals in a given name: sys.sig[[list_of_indices]]

The only thing supported in this PR is ranges with optional upper and lower limits. If you want to index by a list, you would have to either list things out or use a list comprehension, such as [f'sys.sig[{i}]' for i in list_of_indices].

  1. Does this work for name-based auto-connection, that is, no connections keyword, provided all signals of the same name have the same size?

That functionality was there before, since if you had a system with outputs [xd[0], xd[1], ..., xd[n]] and another system with those same signal names as inputs, then they would all connect together by virtue of matching in their enumerated form.

What you can do now that you couldn't do before is connect all of the xd's to new signals zd's (say) by saying something like

connections=[['zd', 'xd']]

@sawyerbfuller
Copy link
Contributor

OK, this looks good. One other question: I recently noticed that there is a bug in the current code in which system names and labels are lost when indexing the system:

e_summer = ct.summing_junction(['r', '-y'], 'e')
print(e_summer.input_labels)
print(e_summer[0,0].input_labels) # loses name and labels
print(e_summer[0,0].output_labels) # loses name and labels

gives

['r', 'y']
['u[0]']
['y[0]']

Any chance this PR addresses that?

@murrayrm
Copy link
Member Author

This PR doesn't address __getitem__ signal naming, but this is a good place to fix that up.

One question is how to handle system naming. When we take a subset of a set of inputs and outputs we probably don't want to use exactly the same system name. Normally, we set the name to something like sysname$linearized or sysname$sampled and allow you to override the name in the function call. We can't do easy renaming here (since you can't add keywords to something like sys[2, [1, 2]]), but we can change the name to something like sys$indexed or sys_[2, [1, 2]] (though the latter might confuse some of our default name parsing, since we assume that names of the form "[\d]" are default system names).

@sawyerbfuller
Copy link
Contributor

sys$indexed seems reasonable to me - keeps the original name but provides indication that it is not the original system.

@sawyerbfuller
Copy link
Contributor

LGTM

@murrayrm murrayrm force-pushed the multivariable_interconnect-30Mar2023 branch 2 times, most recently from 574ae73 to c17a389 Compare June 3, 2023 18:45
@murrayrm murrayrm force-pushed the multivariable_interconnect-30Mar2023 branch from d646017 to 55e2b55 Compare June 7, 2023 04:32
@murrayrm murrayrm merged commit 71bbce2 into python-control:main Jun 10, 2023
14 checks passed
@murrayrm murrayrm added this to the 0.10.0 milestone Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants