From 672f229f48f499530ff39acb0a2cbc2ae4d32c5b Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Thu, 14 May 2020 00:45:23 -0400 Subject: [PATCH 01/12] Made sure interconnectedsystems keeps names of parts. --- control/iosys.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/control/iosys.py b/control/iosys.py index 520a6237c..d06916e29 100644 --- a/control/iosys.py +++ b/control/iosys.py @@ -883,6 +883,7 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], self.syslist_index = {} dt = None nstates = 0; self.state_offset = [] + nstatenames = [] ninputs = 0; self.input_offset = [] noutputs = 0; self.output_offset = [] system_count = 0 @@ -908,6 +909,7 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], self.state_offset.append(nstates) # Keep track of the total number of states, inputs, outputs + nstatenames += [sys.name + statename for statename in list(sys.state_index)] nstates += sys.nstates ninputs += sys.ninputs noutputs += sys.noutputs @@ -916,7 +918,7 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], # TODO: look for duplicated system names self.syslist_index[sys.name] = system_count system_count += 1 - + # Check for duplicate systems or duplicate names sysobj_list = [] sysname_list = [] @@ -928,10 +930,13 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], sysobj_list.append(sys) sysname_list.append(sys.name) + # This might cause a problem if there are duplicate names... + states = states or nstatenames + # Create the I/O system super(InterconnectedSystem, self).__init__( inputs=len(inplist), outputs=len(outlist), - states=nstates, params=params, dt=dt) + states=states, params=params, dt=dt, name=name) # If input or output list was specified, update it nsignals, self.input_index = \ @@ -939,7 +944,7 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], if nsignals is not None and len(inplist) != nsignals: raise ValueError("Wrong number/type of inputs given.") nsignals, self.output_index = \ - self._process_signal_list(outputs, prefix='y') + self._process_signal_list(outputs or len(outlist), prefix='y') if nsignals is not None and len(outlist) != nsignals: raise ValueError("Wrong number/type of outputs given.") From 15a442a7d8c6bd8b49f1bb73731e181530171c46 Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Mon, 18 May 2020 11:09:40 -0400 Subject: [PATCH 02/12] Update control/iosys.py Co-authored-by: Ben Greiner --- control/iosys.py | 1 - 1 file changed, 1 deletion(-) diff --git a/control/iosys.py b/control/iosys.py index d06916e29..784a1f5b9 100644 --- a/control/iosys.py +++ b/control/iosys.py @@ -918,7 +918,6 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], # TODO: look for duplicated system names self.syslist_index[sys.name] = system_count system_count += 1 - # Check for duplicate systems or duplicate names sysobj_list = [] sysname_list = [] From 77f944128d5ce305cc7be7a45ef486369b623bae Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Mon, 18 May 2020 11:48:50 -0400 Subject: [PATCH 03/12] Update control/iosys.py Co-authored-by: Ben Greiner --- control/iosys.py | 1 + 1 file changed, 1 insertion(+) diff --git a/control/iosys.py b/control/iosys.py index 784a1f5b9..61949c979 100644 --- a/control/iosys.py +++ b/control/iosys.py @@ -918,6 +918,7 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], # TODO: look for duplicated system names self.syslist_index[sys.name] = system_count system_count += 1 + # Check for duplicate systems or duplicate names sysobj_list = [] sysname_list = [] From d45a6c08e942ee3e18a093e5969f53738391df01 Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Wed, 20 May 2020 16:10:20 -0400 Subject: [PATCH 04/12] Fixed signal names for InterconnectedSystems. Only the outlist specification I am not sure. First, why can subsystem inputs be connected to system outputs? Then, why was the specification and code in __init__ different from that for the inplist? I changed it to match inplist, but left the weird subsystem input configuration. Check comment that starts with "Convert the output list to a matrix". --- control/iosys.py | 148 +++++++++++++++++++++++------------------------ 1 file changed, 74 insertions(+), 74 deletions(-) diff --git a/control/iosys.py b/control/iosys.py index d06916e29..256450d6e 100644 --- a/control/iosys.py +++ b/control/iosys.py @@ -108,6 +108,14 @@ class for a set of subclasses that are used to implement specific The default is to return the entire system state. """ + + idCounter = 0 + def name_or_default(self, name=None): + if name is None: + name = "sys[{}]".format(InputOutputSystem.idCounter) + InputOutputSystem.idCounter += 1 + return name + def __init__(self, inputs=None, outputs=None, states=None, params={}, dt=None, name=None): """Create an input/output system. @@ -143,7 +151,8 @@ def __init__(self, inputs=None, outputs=None, states=None, params={}, functions for the system as default values, overriding internal defaults. name : string, optional - System name (used for specifying signals) + System name (used for specifying signals). If unspecified, a generic + name is generated with a unique integer id. Returns ------- @@ -152,9 +161,9 @@ def __init__(self, inputs=None, outputs=None, states=None, params={}, """ # Store the input arguments - self.params = params.copy() # default parameters - self.dt = dt # timebase - self.name = name # system name + self.params = params.copy() # default parameters + self.dt = dt # timebase + self.name = self.name_or_default(name) # system name # Parse and store the number of inputs, outputs, and states self.set_inputs(inputs) @@ -204,10 +213,12 @@ def __mul__(sys2, sys1): if dt is False: raise ValueError("System timebases are not compabile") + inplist = [(0,i) for i in range(sys1.ninputs)] + outlist = [(1,i) for i in range(sys2.noutputs)] # Return the series interconnection between the systems - newsys = InterconnectedSystem((sys1, sys2)) + newsys = InterconnectedSystem((sys1, sys2), inplist=inplist, outlist=outlist) - # Set up the connecton map + # Set up the connection map manually newsys.set_connect_map(np.block( [[np.zeros((sys1.ninputs, sys1.noutputs)), np.zeros((sys1.ninputs, sys2.noutputs))], @@ -215,18 +226,6 @@ def __mul__(sys2, sys1): np.zeros((sys2.ninputs, sys2.noutputs))]] )) - # Set up the input map - newsys.set_input_map(np.concatenate( - (np.eye(sys1.ninputs), np.zeros((sys2.ninputs, sys1.ninputs))), - axis=0)) - # TODO: set up input names - - # Set up the output map - newsys.set_output_map(np.concatenate( - (np.zeros((sys2.noutputs, sys1.noutputs)), np.eye(sys2.noutputs)), - axis=1)) - # TODO: set up output names - # Return the newly created system return newsys @@ -271,18 +270,10 @@ def __add__(sys1, sys2): ninputs = sys1.ninputs noutputs = sys1.noutputs + inplist = [[(0,i),(1,i)] for i in range(ninputs)] + outlist = [[(0,i),(1,i)] for i in range(noutputs)] # Create a new system to handle the composition - newsys = InterconnectedSystem((sys1, sys2)) - - # Set up the input map - newsys.set_input_map(np.concatenate( - (np.eye(ninputs), np.eye(ninputs)), axis=0)) - # TODO: set up input names - - # Set up the output map - newsys.set_output_map(np.concatenate( - (np.eye(noutputs), np.eye(noutputs)), axis=1)) - # TODO: set up output names + newsys = InterconnectedSystem((sys1, sys2), inplist=inplist, outlist=outlist) # Return the newly created system return newsys @@ -301,16 +292,10 @@ def __neg__(sys): if sys.ninputs is None or sys.noutputs is None: raise ValueError("Can't determine number of inputs or outputs") + inplist = [(0,i) for i in range(sys.ninputs)] + outlist = [(0,i,-1) for i in range(sys.noutputs)] # Create a new system to hold the negation - newsys = InterconnectedSystem((sys,), dt=sys.dt) - - # Set up the input map (identity) - newsys.set_input_map(np.eye(sys.ninputs)) - # TODO: set up input names - - # Set up the output map (negate the output) - newsys.set_output_map(-np.eye(sys.noutputs)) - # TODO: set up output names + newsys = InterconnectedSystem((sys,), dt=sys.dt, inplist=inplist, outlist=outlist) # Return the newly created system return newsys @@ -482,10 +467,13 @@ def feedback(self, other=1, sign=-1, params={}): if dt is False: raise ValueError("System timebases are not compabile") + inplist = [(0,i) for i in range(self.ninputs)] + outlist = [(0,i) for i in range(self.noutputs)] # Return the series interconnection between the systems - newsys = InterconnectedSystem((self, other), params=params, dt=dt) + newsys = InterconnectedSystem((self, other), inplist=inplist, outlist=outlist, + params=params, dt=dt) - # Set up the connecton map + # Set up the connecton map manually newsys.set_connect_map(np.block( [[np.zeros((self.ninputs, self.noutputs)), sign * np.eye(self.ninputs, other.noutputs)], @@ -493,18 +481,6 @@ def feedback(self, other=1, sign=-1, params={}): np.zeros((other.ninputs, other.noutputs))]] )) - # Set up the input map - newsys.set_input_map(np.concatenate( - (np.eye(self.ninputs), np.zeros((other.ninputs, self.ninputs))), - axis=0)) - # TODO: set up input names - - # Set up the output map - newsys.set_output_map(np.concatenate( - (np.eye(self.noutputs), np.zeros((self.noutputs, other.noutputs))), - axis=1)) - # TODO: set up output names - # Return the newly created system return newsys @@ -566,7 +542,9 @@ def linearize(self, x0, u0, t=0, params={}, eps=1e-6): def copy(self): """Make a copy of an input/output system.""" - return copy.copy(self) + newsys = copy.copy(self) + newsys.name = self.name_or_default(None) + return newsys class LinearIOSystem(InputOutputSystem, StateSpace): @@ -808,10 +786,13 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], syslist : array_like of InputOutputSystems The list of input/output systems to be connected - connections : tuple of connection specifications, optional + connections : list of tuple of connection specifications, optional Description of the internal connections between the subsystems. - Each element of the tuple describes an input to one of the - subsystems. The entries are are of the form: + + [connection1, connection2, ...] + + Each connection is a tuple that describes an input to one of the + subsystems. The entries are of the form: (input-spec, output-spec1, output-spec2, ...) @@ -835,10 +816,15 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], If omitted, the connection map (matrix) can be specified using the :func:`~control.InterconnectedSystem.set_connect_map` method. - inplist : tuple of input specifications, optional + inplist : List of tuple of input specifications, optional List of specifications for how the inputs for the overall system are mapped to the subsystem inputs. The input specification is - the same as the form defined in the connection specification. + similar to the form defined in the connection specification, except + that connections do not specify an input-spec, since these are + the system inputs. The entries are thus of the form: + + (output-spec1, output-spec2, ...) + Each system input is added to the input for the listed subsystem. If omitted, the input map can be specified using the @@ -847,7 +833,7 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], outlist : tuple of output specifications, optional List of specifications for how the outputs for the subsystems are mapped to overall system outputs. The output specification is the - same as the form defined in the connection specification + same as the form defined in the inplist specification (including the optional gain term). Numbered outputs must be chosen from the list of subsystem outputs, but named outputs can also be contained in the list of subsystem inputs. @@ -883,7 +869,6 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], self.syslist_index = {} dt = None nstates = 0; self.state_offset = [] - nstatenames = [] ninputs = 0; self.input_offset = [] noutputs = 0; self.output_offset = [] system_count = 0 @@ -909,7 +894,6 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], self.state_offset.append(nstates) # Keep track of the total number of states, inputs, outputs - nstatenames += [sys.name + statename for statename in list(sys.state_index)] nstates += sys.nstates ninputs += sys.ninputs noutputs += sys.noutputs @@ -927,11 +911,17 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], warn("Duplicate object found in system list: %s" % str(sys)) elif sys.name is not None and sys.name in sysname_list: warn("Duplicate name found in system list: %s" % sys.name) + sysobj_list.append(sys) sysname_list.append(sys.name) - # This might cause a problem if there are duplicate names... - states = states or nstatenames + # This won't work when there are duplicate sysnames + # Should we throw an error rather than just warning above? + # Users can use sys.copy() to get a unique name for the dup system. + if states is None: + states = [] + for sys in sysobj_list: + states += [sys.name + '.' + statename for statename in sys.state_index.keys()] # Create the I/O system super(InterconnectedSystem, self).__init__( @@ -939,14 +929,16 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], states=states, params=params, dt=dt, name=name) # If input or output list was specified, update it - nsignals, self.input_index = \ - self._process_signal_list(inputs, prefix='u') - if nsignals is not None and len(inplist) != nsignals: - raise ValueError("Wrong number/type of inputs given.") - nsignals, self.output_index = \ - self._process_signal_list(outputs or len(outlist), prefix='y') - if nsignals is not None and len(outlist) != nsignals: - raise ValueError("Wrong number/type of outputs given.") + if inputs is not None: + nsignals, self.input_index = \ + self._process_signal_list(inputs, prefix='u') + if nsignals is not None and len(inplist) != nsignals: + raise ValueError("Wrong number/type of inputs given.") + if outputs is not None: + nsignals, self.output_index = \ + self._process_signal_list(outputs, prefix='y') + if nsignals is not None and len(outlist) != nsignals: + raise ValueError("Wrong number/type of outputs given.") # Convert the list of interconnections to a connection map (matrix) self.connect_map = np.zeros((ninputs, noutputs)) @@ -964,10 +956,18 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], self.input_map[self._parse_input_spec(spec), index] = 1 # Convert the output list to a matrix: maps subsystems to system + # why can subsystem inputs map to outputs? shouldn't it be + # system inputs (hence self.ninputs instead of ninputs)? self.output_map = np.zeros((self.noutputs, noutputs + ninputs)) - for index in range(len(outlist)): - ylist_index, gain = self._parse_output_spec(outlist[index]) - self.output_map[index, ylist_index] = gain + for index, outspec in enumerate(outlist): + if isinstance(outspec, (int, str, tuple)): outspec = [outspec] + for spec in outspec: + ylist_index, gain = self._parse_output_spec(spec) + self.output_map[index, ylist_index] = gain + # self.output_map = np.zeros((self.noutputs, noutputs + ninputs)) + # for index in range(len(outlist)): + # ylist_index, gain = self._parse_output_spec(outlist[index]) + # self.output_map[index, ylist_index] = gain # Save the parameters for the system self.params = params.copy() From c8fba9718469abe2d1fe086d7397804e08655449 Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Wed, 15 Jul 2020 15:34:13 -0400 Subject: [PATCH 05/12] Update control/iosys.py Co-authored-by: Ben Greiner --- control/iosys.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/control/iosys.py b/control/iosys.py index 98378fcf3..c4f98453e 100644 --- a/control/iosys.py +++ b/control/iosys.py @@ -964,10 +964,6 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], for spec in outspec: ylist_index, gain = self._parse_output_spec(spec) self.output_map[index, ylist_index] = gain - # self.output_map = np.zeros((self.noutputs, noutputs + ninputs)) - # for index in range(len(outlist)): - # ylist_index, gain = self._parse_output_spec(outlist[index]) - # self.output_map[index, ylist_index] = gain # Save the parameters for the system self.params = params.copy() From bd38dea13f8aae265b256482b02d063e00f2f672 Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Wed, 15 Jul 2020 15:35:37 -0400 Subject: [PATCH 06/12] Update control/iosys.py Co-authored-by: Ben Greiner --- control/iosys.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/control/iosys.py b/control/iosys.py index c4f98453e..4c2fd18f0 100644 --- a/control/iosys.py +++ b/control/iosys.py @@ -956,8 +956,6 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], self.input_map[self._parse_input_spec(spec), index] = 1 # Convert the output list to a matrix: maps subsystems to system - # why can subsystem inputs map to outputs? shouldn't it be - # system inputs (hence self.ninputs instead of ninputs)? self.output_map = np.zeros((self.noutputs, noutputs + ninputs)) for index, outspec in enumerate(outlist): if isinstance(outspec, (int, str, tuple)): outspec = [outspec] From 35676f27d65afd88211de413651b1d2eea404204 Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Wed, 15 Jul 2020 20:51:57 -0400 Subject: [PATCH 07/12] Fixed some naming convention problems and added unit tests for naming conventions. --- control/iosys.py | 60 +++++++++++++++++++++++++---------- control/tests/iosys_test.py | 62 ++++++++++++++++++++++++++++++++++--- 2 files changed, 101 insertions(+), 21 deletions(-) diff --git a/control/iosys.py b/control/iosys.py index 98378fcf3..362ca3aeb 100644 --- a/control/iosys.py +++ b/control/iosys.py @@ -76,7 +76,8 @@ class for a set of subclasses that are used to implement specific Parameter values for the systems. Passed to the evaluation functions for the system as default values, overriding internal defaults. name : string, optional - System name (used for specifying signals) + System name (used for specifying signals). If unspecified, a generic + name is generated with a unique integer id. Attributes ---------- @@ -540,10 +541,10 @@ def linearize(self, x0, u0, t=0, params={}, eps=1e-6): linsys = StateSpace(A, B, C, D, self.dt, remove_useless=False) return LinearIOSystem(linsys) - def copy(self): + def copy(self, newname=None): """Make a copy of an input/output system.""" newsys = copy.copy(self) - newsys.name = self.name_or_default(None) + newsys.name = self.name_or_default(newname) return newsys @@ -588,7 +589,8 @@ def __init__(self, linsys, inputs=None, outputs=None, states=None, functions for the system as default values, overriding internal defaults. name : string, optional - System name (used for specifying signals) + System name (used for specifying signals). If unspecified, a generic + name is generated with a unique integer id. Returns ------- @@ -706,7 +708,8 @@ def __init__(self, updfcn, outfcn=None, inputs=None, outputs=None, * dt = True Discrete time with unspecified sampling time name : string, optional - System name (used for specifying signals). + System name (used for specifying signals). If unspecified, a generic + name is generated with a unique integer id. Returns ------- @@ -841,6 +844,23 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], If omitted, the output map can be specified using the `set_output_map` method. + inputs : int, list of str or None, optional + Description of the system inputs. This can be given as an integer + count or as a list of strings that name the individual signals. + If an integer count is specified, the names of the signal will be + of the form `s[i]` (where `s` is one of `u`, `y`, or `x`). If + this parameter is not given or given as `None`, the relevant + quantity will be determined when possible based on other + information provided to functions using the system. + + outputs : int, list of str or None, optional + Description of the system outputs. Same format as `inputs`. + + states : int, list of str, or None, optional + Description of the system states. Same format as `inputs`, except + the state names will be of the form '.', + for each subsys in syslist and each state_name of each subsys. + params : dict, optional Parameter values for the systems. Passed to the evaluation functions for the system as default values, overriding internal @@ -857,7 +877,8 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], * dt = True Discrete time with unspecified sampling time name : string, optional - System name (used for specifying signals). + System name (used for specifying signals). If unspecified, a generic + name is generated with a unique integer id. """ # Convert input and output names to lists if they aren't already @@ -904,24 +925,29 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], system_count += 1 # Check for duplicate systems or duplicate names - sysobj_list = [] - sysname_list = [] + # Duplicates are renamed sysname_1, sysname_2, etc. + sysobj_name_dct = {} + sysname_count_dct = {} for sys in syslist: - if sys in sysobj_list: - warn("Duplicate object found in system list: %s" % str(sys)) - elif sys.name is not None and sys.name in sysname_list: - warn("Duplicate name found in system list: %s" % sys.name) + if sys in sysobj_name_dct: + raise ValueError("Duplicate object found in system list: %s" % str(sys)) + elif sys.name is not None and sys.name in sysname_count_dct: + count = sysname_count_dct[sys.name] + sysname_count_dct[sys.name] += 1 + sysname = sys.name + "_" + str(count) + sysobj_name_dct[sys] = sysname + warn("Duplicate name found in system list. Renamed to {}".format(sysname)) + else: + sysname_count_dct[sys.name] = 1 + sysobj_name_dct[sys] = sys.name - sysobj_list.append(sys) - sysname_list.append(sys.name) - # This won't work when there are duplicate sysnames # Should we throw an error rather than just warning above? # Users can use sys.copy() to get a unique name for the dup system. if states is None: states = [] - for sys in sysobj_list: - states += [sys.name + '.' + statename for statename in sys.state_index.keys()] + for sys, name in sysobj_name_dct.items(): + states += [name + '.' + statename for statename in sys.state_index.keys()] # Create the I/O system super(InterconnectedSystem, self).__init__( diff --git a/control/tests/iosys_test.py b/control/tests/iosys_test.py index 27651de71..8e2af924b 100644 --- a/control/tests/iosys_test.py +++ b/control/tests/iosys_test.py @@ -802,10 +802,64 @@ def test_named_signals(self): ) ss_feedback = ct.feedback(self.mimo_linsys1, self.mimo_linsys2) lin_feedback = ct.linearize(ios_connect, 0, 0) - np.testing.assert_array_almost_equal(ss_feedback.A, lin_feedback.A) - np.testing.assert_array_almost_equal(ss_feedback.B, lin_feedback.B) - np.testing.assert_array_almost_equal(ss_feedback.C, lin_feedback.C) - np.testing.assert_array_almost_equal(ss_feedback.D, lin_feedback.D) + for M, N in ((ss_feedback.A, lin_feedback.A), (ss_feedback.B, lin_feedback.B), + (ss_feedback.C, lin_feedback.C), (ss_feedback.D, lin_feedback.D)): + np.testing.assert_array_almost_equal(M, N) + + # We also enforce generic names to be present when systems are created + # without names (sys, states, inputs and outputs) + def check_name_and_ios_counts(sys): + self.assertIsNotNone(sys.name) + self.assertEqual(len(sys.state_index), sys.nstates) + self.assertEqual(len(sys.input_index), sys.ninputs) + self.assertEqual(len(sys.output_index), sys.noutputs) + + sys = ct.LinearIOSystem(self.mimo_linsys1) + check_name_and_ios_counts(sys) + + nlios222 = ct.NonlinearIOSystem( + lambda t,x,u,params: x, inputs=2, outputs=2, states=2 + ) + nlios202 = ct.NonlinearIOSystem( + None, lambda t,x,u,params: u, inputs=2, outputs=2 + ) + check_name_and_ios_counts(nlios202) + + # Unnamed/unnamed connections + uu_series = nlios222 * nlios202 + uu_parallel = nlios222 + nlios202 + u_neg = - nlios222 + uu_feedback = nlios202.feedback(nlios222) + uu_dup = nlios222 * nlios222.copy() + uu_hierarchical = uu_series*nlios222 + + check_name_and_ios_counts(uu_series) + check_name_and_ios_counts(uu_parallel) + check_name_and_ios_counts(u_neg) + check_name_and_ios_counts(uu_feedback) + check_name_and_ios_counts(uu_dup) + check_name_and_ios_counts(uu_hierarchical) + + # Unnamed/named connections + un_series = nlios222 * sys1 + un_parallel = nlios222 + sys1 + un_feedback = nlios202.feedback(sys1) + un_dup = nlios222 * sys1.copy() + un_hierarchical = uu_series*nlios222 + + check_name_and_ios_counts(un_series) + check_name_and_ios_counts(un_parallel) + check_name_and_ios_counts(un_feedback) + check_name_and_ios_counts(un_dup) + check_name_and_ios_counts(un_hierarchical) + + # Name conflict (test new names are assigned) + same_name_nlios222 = nlios222.copy(nlios222.name) + same_name_sum = nlios222 + same_name_nlios222 + check_name_and_ios_counts(same_name_sum) + + # Same system conflict + self.assertRaises(ValueError, lambda: nlios222*nlios222) def test_lineariosys_statespace(self): """Make sure that a LinearIOSystem is also a StateSpace object""" From 69cc8c559f30ce4f5ebec3768f137537222337ac Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Sun, 19 Jul 2020 14:40:34 -0400 Subject: [PATCH 08/12] Removed for loops from tests to get better error messages --- control/tests/iosys_test.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/control/tests/iosys_test.py b/control/tests/iosys_test.py index 8e2af924b..d1199b20c 100644 --- a/control/tests/iosys_test.py +++ b/control/tests/iosys_test.py @@ -763,17 +763,19 @@ def test_named_signals(self): ios_mul = sys1 * sys2 ss_series = self.mimo_linsys1 * self.mimo_linsys2 lin_series = ct.linearize(ios_mul, 0, 0) - for M, N in ((ss_series.A, lin_series.A), (ss_series.B, lin_series.B), - (ss_series.C, lin_series.C), (ss_series.D, lin_series.D)): - np.testing.assert_array_almost_equal(M, N) + np.testing.assert_array_almost_equal(ss_series.A, lin_series.A) + np.testing.assert_array_almost_equal(ss_series.B, lin_series.B) + np.testing.assert_array_almost_equal(ss_series.C, lin_series.C) + np.testing.assert_array_almost_equal(ss_series.D, lin_series.D) # Series interconnection (sys1 * sys2) using series ios_series = ct.series(sys2, sys1) ss_series = ct.series(self.mimo_linsys2, self.mimo_linsys1) lin_series = ct.linearize(ios_series, 0, 0) - for M, N in ((ss_series.A, lin_series.A), (ss_series.B, lin_series.B), - (ss_series.C, lin_series.C), (ss_series.D, lin_series.D)): - np.testing.assert_array_almost_equal(M, N) + np.testing.assert_array_almost_equal(ss_series.A, lin_series.A) + np.testing.assert_array_almost_equal(ss_series.B, lin_series.B) + np.testing.assert_array_almost_equal(ss_series.C, lin_series.C) + np.testing.assert_array_almost_equal(ss_series.D, lin_series.D) # Series interconnection (sys1 * sys2) using named + mixed signals ios_connect = ios.InterconnectedSystem( @@ -786,9 +788,10 @@ def test_named_signals(self): outlist=((1, 'y[0]'), 'sys1.y[1]') ) lin_series = ct.linearize(ios_connect, 0, 0) - for M, N in ((ss_series.A, lin_series.A), (ss_series.B, lin_series.B), - (ss_series.C, lin_series.C), (ss_series.D, lin_series.D)): - np.testing.assert_array_almost_equal(M, N) + np.testing.assert_array_almost_equal(ss_series.A, lin_series.A) + np.testing.assert_array_almost_equal(ss_series.B, lin_series.B) + np.testing.assert_array_almost_equal(ss_series.C, lin_series.C) + np.testing.assert_array_almost_equal(ss_series.D, lin_series.D) # Make sure that we can use input signal names as system outputs ios_connect = ios.InterconnectedSystem( @@ -802,9 +805,10 @@ def test_named_signals(self): ) ss_feedback = ct.feedback(self.mimo_linsys1, self.mimo_linsys2) lin_feedback = ct.linearize(ios_connect, 0, 0) - for M, N in ((ss_feedback.A, lin_feedback.A), (ss_feedback.B, lin_feedback.B), - (ss_feedback.C, lin_feedback.C), (ss_feedback.D, lin_feedback.D)): - np.testing.assert_array_almost_equal(M, N) + np.testing.assert_array_almost_equal(ss_feedback.A, lin_feedback.A) + np.testing.assert_array_almost_equal(ss_feedback.B, lin_feedback.B) + np.testing.assert_array_almost_equal(ss_feedback.C, lin_feedback.C) + np.testing.assert_array_almost_equal(ss_feedback.D, lin_feedback.D) # We also enforce generic names to be present when systems are created # without names (sys, states, inputs and outputs) From e76a4f8ec9ad30e3d7fc6baacd269013c65abbed Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Sun, 19 Jul 2020 16:09:31 -0400 Subject: [PATCH 09/12] Update control/iosys.py Co-authored-by: Ben Greiner --- control/iosys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control/iosys.py b/control/iosys.py index 7e3a58962..df8d1c4c3 100644 --- a/control/iosys.py +++ b/control/iosys.py @@ -544,7 +544,7 @@ def linearize(self, x0, u0, t=0, params={}, eps=1e-6): def copy(self, newname=None): """Make a copy of an input/output system.""" newsys = copy.copy(self) - newsys.name = self.name_or_default(newname) + newsys.name = self.name_or_default("copy of " + self.name if not newname) return newsys From abb1f19176e13a1a7e5f608282b9235b626eb220 Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Sun, 19 Jul 2020 16:11:11 -0400 Subject: [PATCH 10/12] Changed a lot of naming convention code/unit test following pull-request conversation. --- control/iosys.py | 30 +++++----- control/tests/iosys_test.py | 107 ++++++++++++++++++++++-------------- 2 files changed, 80 insertions(+), 57 deletions(-) diff --git a/control/iosys.py b/control/iosys.py index 7e3a58962..0ad9d6331 100644 --- a/control/iosys.py +++ b/control/iosys.py @@ -892,8 +892,9 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], nstates = 0; self.state_offset = [] ninputs = 0; self.input_offset = [] noutputs = 0; self.output_offset = [] - system_count = 0 - for sys in syslist: + sysobj_name_dct = {} + sysname_count_dct = {} + for sysidx, sys in enumerate(syslist): # Make sure time bases are consistent # TODO: Use lti._find_timebase() instead? if dt is None and sys.dt is not None: @@ -919,35 +920,30 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], ninputs += sys.ninputs noutputs += sys.noutputs - # Store the index to the system for later retrieval - # TODO: look for duplicated system names - self.syslist_index[sys.name] = system_count - system_count += 1 - - # Check for duplicate systems or duplicate names - # Duplicates are renamed sysname_1, sysname_2, etc. - sysobj_name_dct = {} - sysname_count_dct = {} - for sys in syslist: + # Check for duplicate systems or duplicate names + # Duplicates are renamed sysname_1, sysname_2, etc. if sys in sysobj_name_dct: - raise ValueError("Duplicate object found in system list: %s" % str(sys)) - elif sys.name is not None and sys.name in sysname_count_dct: + sys = sys.copy() + warn("Duplicate object found in system list: %s. Making a copy" % str(sys)) + if sys.name is not None and sys.name in sysname_count_dct: count = sysname_count_dct[sys.name] sysname_count_dct[sys.name] += 1 sysname = sys.name + "_" + str(count) sysobj_name_dct[sys] = sysname + self.syslist_index[sysname] = sysidx warn("Duplicate name found in system list. Renamed to {}".format(sysname)) else: sysname_count_dct[sys.name] = 1 sysobj_name_dct[sys] = sys.name - + self.syslist_index[sys.name] = sysidx + # This won't work when there are duplicate sysnames # Should we throw an error rather than just warning above? # Users can use sys.copy() to get a unique name for the dup system. if states is None: states = [] - for sys, name in sysobj_name_dct.items(): - states += [name + '.' + statename for statename in sys.state_index.keys()] + for sys, sysname in sysobj_name_dct.items(): + states += [sysname + '.' + statename for statename in sys.state_index.keys()] # Create the I/O system super(InterconnectedSystem, self).__init__( diff --git a/control/tests/iosys_test.py b/control/tests/iosys_test.py index d1199b20c..20a0ad2ce 100644 --- a/control/tests/iosys_test.py +++ b/control/tests/iosys_test.py @@ -810,60 +810,87 @@ def test_named_signals(self): np.testing.assert_array_almost_equal(ss_feedback.C, lin_feedback.C) np.testing.assert_array_almost_equal(ss_feedback.D, lin_feedback.D) - # We also enforce generic names to be present when systems are created - # without names (sys, states, inputs and outputs) - def check_name_and_ios_counts(sys): - self.assertIsNotNone(sys.name) - self.assertEqual(len(sys.state_index), sys.nstates) - self.assertEqual(len(sys.input_index), sys.ninputs) - self.assertEqual(len(sys.output_index), sys.noutputs) + def test_sys_naming_convention(self): + """Enforce generic system names 'sys[i]' to be present when systems are created + without explicit names.""" + ct.InputOutputSystem.idCounter = 0 sys = ct.LinearIOSystem(self.mimo_linsys1) - check_name_and_ios_counts(sys) - - nlios222 = ct.NonlinearIOSystem( + self.assertEquals(sys.name, "sys[0]") + + namedsys = ios.NonlinearIOSystem( + updfcn = lambda t, x, u, params: np.array( + np.dot(self.mimo_linsys1.A, np.reshape(x, (-1, 1))) \ + + np.dot(self.mimo_linsys1.B, np.reshape(u, (-1, 1))) + ).reshape(-1,), + outfcn = lambda t, x, u, params: np.array( + self.mimo_linsys1.C * np.reshape(x, (-1, 1)) \ + + self.mimo_linsys1.D * np.reshape(u, (-1, 1)) + ).reshape(-1,), + inputs = ('u[0]', 'u[1]'), + outputs = ('y[0]', 'y[1]'), + states = self.mimo_linsys1.states, + name = 'namedsys') + unnamedsys1 = ct.NonlinearIOSystem( lambda t,x,u,params: x, inputs=2, outputs=2, states=2 ) - nlios202 = ct.NonlinearIOSystem( + unnamedsys2 = ct.NonlinearIOSystem( None, lambda t,x,u,params: u, inputs=2, outputs=2 ) - check_name_and_ios_counts(nlios202) + breakpoint() + self.assertEquals(unnamedsys2.name, "sys[2]") # Unnamed/unnamed connections - uu_series = nlios222 * nlios202 - uu_parallel = nlios222 + nlios202 - u_neg = - nlios222 - uu_feedback = nlios202.feedback(nlios222) - uu_dup = nlios222 * nlios222.copy() - uu_hierarchical = uu_series*nlios222 - - check_name_and_ios_counts(uu_series) - check_name_and_ios_counts(uu_parallel) - check_name_and_ios_counts(u_neg) - check_name_and_ios_counts(uu_feedback) - check_name_and_ios_counts(uu_dup) - check_name_and_ios_counts(uu_hierarchical) + uu_series = unnamedsys1 * unnamedsys2 + uu_parallel = unnamedsys1 + unnamedsys2 + u_neg = - unnamedsys1 + uu_feedback = unnamedsys2.feedback(unnamedsys1) + uu_dup = unnamedsys1 * unnamedsys1.copy() + uu_hierarchical = uu_series*unnamedsys1 + + self.assertEquals(uu_series.name, "sys[3]") + self.assertEquals(uu_parallel.name, "sys[4]") + self.assertEquals(u_neg.name, "sys[5]") + self.assertEquals(uu_feedback.name, "sys[6]") + self.assertEquals(uu_dup.name, "sys[8]") + self.assertEquals(uu_hierarchical.name, "sys[9]") # Unnamed/named connections - un_series = nlios222 * sys1 - un_parallel = nlios222 + sys1 - un_feedback = nlios202.feedback(sys1) - un_dup = nlios222 * sys1.copy() - un_hierarchical = uu_series*nlios222 - - check_name_and_ios_counts(un_series) - check_name_and_ios_counts(un_parallel) - check_name_and_ios_counts(un_feedback) - check_name_and_ios_counts(un_dup) - check_name_and_ios_counts(un_hierarchical) + un_series = unnamedsys1 * namedsys + un_parallel = unnamedsys1 + namedsys + un_feedback = unnamedsys2.feedback(namedsys) + un_dup = unnamedsys1 * namedsys.copy() + un_hierarchical = uu_series*unnamedsys1 + + self.assertEquals(un_series.name, "sys[10]") + self.assertEquals(un_parallel.name, "sys[11]") + self.assertEquals(un_feedback.name, "sys[12]") + self.assertEquals(un_dup.name, "sys[14]") + self.assertEquals(un_hierarchical.name, "sys[15]") # Name conflict (test new names are assigned) - same_name_nlios222 = nlios222.copy(nlios222.name) - same_name_sum = nlios222 + same_name_nlios222 - check_name_and_ios_counts(same_name_sum) + same_name_sum = unnamedsys1 + unnamedsys1.copy(unnamedsys1.name) + self.assertEquals(same_name_sum.name, "sys[16]") # Same system conflict - self.assertRaises(ValueError, lambda: nlios222*nlios222) + with warnings.catch_warnings(record=True) as warnval: + # Trigger a warning + unnamedsys1 * unnamedsys1 + # Verify that we got a warning + self.assertEqual(len(warnval), 1) + + def test_signals_naming_convention(self): + """Enforce generic names to be present when systems are created + without explicit signal names: + input: 'u[i]' + state: 'x[i]' + output: 'y[i]' + """ + sys = ct.LinearIOSystem(self.mimo_linsys1) + self.assertEqual(len(sys.state_index), sys.nstates) + self.assertEqual(len(sys.input_index), sys.ninputs) + self.assertEqual(len(sys.output_index), sys.noutputs) + def test_lineariosys_statespace(self): """Make sure that a LinearIOSystem is also a StateSpace object""" From 3ec2f025505a9247e5d8a6449a2728977cd50c8a Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Sun, 19 Jul 2020 16:39:54 -0400 Subject: [PATCH 11/12] Fixed system naming conventions and unit tests. Still need to do signal names. --- control/iosys.py | 5 +---- control/tests/iosys_test.py | 35 +++++++++++++++++------------------ control/timeresp.py | 4 ++-- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/control/iosys.py b/control/iosys.py index ab28d6d32..6f4b456c9 100644 --- a/control/iosys.py +++ b/control/iosys.py @@ -544,7 +544,7 @@ def linearize(self, x0, u0, t=0, params={}, eps=1e-6): def copy(self, newname=None): """Make a copy of an input/output system.""" newsys = copy.copy(self) - newsys.name = self.name_or_default("copy of " + self.name if not newname) + newsys.name = self.name_or_default("copy of " + self.name if not newname else newname) return newsys @@ -937,9 +937,6 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[], sysobj_name_dct[sys] = sys.name self.syslist_index[sys.name] = sysidx - # This won't work when there are duplicate sysnames - # Should we throw an error rather than just warning above? - # Users can use sys.copy() to get a unique name for the dup system. if states is None: states = [] for sys, sysname in sysobj_name_dct.items(): diff --git a/control/tests/iosys_test.py b/control/tests/iosys_test.py index 20a0ad2ce..697957deb 100644 --- a/control/tests/iosys_test.py +++ b/control/tests/iosys_test.py @@ -817,7 +817,8 @@ def test_sys_naming_convention(self): ct.InputOutputSystem.idCounter = 0 sys = ct.LinearIOSystem(self.mimo_linsys1) self.assertEquals(sys.name, "sys[0]") - + self.assertEquals(sys.copy().name, "copy of sys[0]") + namedsys = ios.NonlinearIOSystem( updfcn = lambda t, x, u, params: np.array( np.dot(self.mimo_linsys1.A, np.reshape(x, (-1, 1))) \ @@ -837,7 +838,6 @@ def test_sys_naming_convention(self): unnamedsys2 = ct.NonlinearIOSystem( None, lambda t,x,u,params: u, inputs=2, outputs=2 ) - breakpoint() self.assertEquals(unnamedsys2.name, "sys[2]") # Unnamed/unnamed connections @@ -852,8 +852,8 @@ def test_sys_naming_convention(self): self.assertEquals(uu_parallel.name, "sys[4]") self.assertEquals(u_neg.name, "sys[5]") self.assertEquals(uu_feedback.name, "sys[6]") - self.assertEquals(uu_dup.name, "sys[8]") - self.assertEquals(uu_hierarchical.name, "sys[9]") + self.assertEquals(uu_dup.name, "sys[7]") + self.assertEquals(uu_hierarchical.name, "sys[8]") # Unnamed/named connections un_series = unnamedsys1 * namedsys @@ -862,21 +862,15 @@ def test_sys_naming_convention(self): un_dup = unnamedsys1 * namedsys.copy() un_hierarchical = uu_series*unnamedsys1 - self.assertEquals(un_series.name, "sys[10]") - self.assertEquals(un_parallel.name, "sys[11]") - self.assertEquals(un_feedback.name, "sys[12]") - self.assertEquals(un_dup.name, "sys[14]") - self.assertEquals(un_hierarchical.name, "sys[15]") - - # Name conflict (test new names are assigned) - same_name_sum = unnamedsys1 + unnamedsys1.copy(unnamedsys1.name) - self.assertEquals(same_name_sum.name, "sys[16]") + self.assertEquals(un_series.name, "sys[9]") + self.assertEquals(un_parallel.name, "sys[10]") + self.assertEquals(un_feedback.name, "sys[11]") + self.assertEquals(un_dup.name, "sys[12]") + self.assertEquals(un_hierarchical.name, "sys[13]") # Same system conflict with warnings.catch_warnings(record=True) as warnval: - # Trigger a warning unnamedsys1 * unnamedsys1 - # Verify that we got a warning self.assertEqual(len(warnval), 1) def test_signals_naming_convention(self): @@ -945,8 +939,9 @@ def test_lineariosys_statespace(self): np.testing.assert_array_equal(io_feedback.D, ss_feedback.D) def test_duplicates(self): - nlios = ios.NonlinearIOSystem(None, \ - lambda t, x, u, params: u*u, inputs=1, outputs=1) + nlios = ios.NonlinearIOSystem(lambda t,x,u,params: x, \ + lambda t, x, u, params: u*u, \ + inputs=1, outputs=1, states=1, name="sys") # Turn off deprecation warnings warnings.simplefilter("ignore", category=DeprecationWarning) @@ -967,7 +962,11 @@ def test_duplicates(self): nlios2 = nlios.copy() with warnings.catch_warnings(record=True) as warnval: ios_series = nlios1 * nlios2 - self.assertEqual(len(warnval), 0) + self.assertEquals(len(warnval), 1) + # when subsystems have the same name, duplicates are + # renamed + self.assertTrue("copy of sys_1.x[0]" in ios_series.state_index.keys()) + self.assertTrue("copy of sys.x[0]" in ios_series.state_index.keys()) # Duplicate names iosys_siso = ct.LinearIOSystem(self.siso_linsys) diff --git a/control/timeresp.py b/control/timeresp.py index 0521fcc74..d1588b93f 100644 --- a/control/timeresp.py +++ b/control/timeresp.py @@ -404,8 +404,8 @@ def forced_response(sys, T=None, U=0., X0=0., transpose=False, xout = xout[::inc, :] # Transpose the output and state vectors to match local convention - xout = sp.transpose(xout) - yout = sp.transpose(yout) + xout = np.transpose(xout) + yout = np.transpose(yout) # Get rid of unneeded dimensions if squeeze: From b2c8d44ed1d08439336b705b292920f6d6da210e Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Mon, 20 Jul 2020 17:13:05 -0400 Subject: [PATCH 12/12] Finished test_signals_naming_convention unit test --- control/tests/iosys_test.py | 60 +++++++++++++++++++++++++++++++------ 1 file changed, 51 insertions(+), 9 deletions(-) diff --git a/control/tests/iosys_test.py b/control/tests/iosys_test.py index 561fa72f6..9bc03ca20 100644 --- a/control/tests/iosys_test.py +++ b/control/tests/iosys_test.py @@ -820,14 +820,8 @@ def test_sys_naming_convention(self): self.assertEquals(sys.copy().name, "copy of sys[0]") namedsys = ios.NonlinearIOSystem( - updfcn = lambda t, x, u, params: np.array( - np.dot(self.mimo_linsys1.A, np.reshape(x, (-1, 1))) \ - + np.dot(self.mimo_linsys1.B, np.reshape(u, (-1, 1))) - ).reshape(-1,), - outfcn = lambda t, x, u, params: np.array( - self.mimo_linsys1.C * np.reshape(x, (-1, 1)) \ - + self.mimo_linsys1.D * np.reshape(u, (-1, 1)) - ).reshape(-1,), + updfcn = lambda t, x, u, params: x, + outfcn = lambda t, x, u, params: u, inputs = ('u[0]', 'u[1]'), outputs = ('y[0]', 'y[1]'), states = self.mimo_linsys1.states, @@ -880,11 +874,59 @@ def test_signals_naming_convention(self): state: 'x[i]' output: 'y[i]' """ + ct.InputOutputSystem.idCounter = 0 sys = ct.LinearIOSystem(self.mimo_linsys1) + for statename in ["x[0]", "x[1]"]: + self.assertTrue(statename in sys.state_index) + for inputname in ["u[0]", "u[1]"]: + self.assertTrue(inputname in sys.input_index) + for outputname in ["y[0]", "y[1]"]: + self.assertTrue(outputname in sys.output_index) self.assertEqual(len(sys.state_index), sys.nstates) self.assertEqual(len(sys.input_index), sys.ninputs) self.assertEqual(len(sys.output_index), sys.noutputs) - + + namedsys = ios.NonlinearIOSystem( + updfcn = lambda t, x, u, params: x, + outfcn = lambda t, x, u, params: u, + inputs = ('u0'), + outputs = ('y0'), + states = ('x0'), + name = 'namedsys') + unnamedsys = ct.NonlinearIOSystem( + lambda t,x,u,params: x, inputs=1, outputs=1, states=1 + ) + self.assertTrue('u0' in namedsys.input_index) + self.assertTrue('y0' in namedsys.output_index) + self.assertTrue('x0' in namedsys.state_index) + + # Unnamed/named connections + un_series = unnamedsys * namedsys + un_parallel = unnamedsys + namedsys + un_feedback = unnamedsys.feedback(namedsys) + un_dup = unnamedsys * namedsys.copy() + un_hierarchical = un_series*unnamedsys + u_neg = - unnamedsys + + self.assertTrue("sys[1].x[0]" in un_series.state_index) + self.assertTrue("namedsys.x0" in un_series.state_index) + self.assertTrue("sys[1].x[0]" in un_parallel.state_index) + self.assertTrue("namedsys.x0" in un_series.state_index) + self.assertTrue("sys[1].x[0]" in un_feedback.state_index) + self.assertTrue("namedsys.x0" in un_feedback.state_index) + self.assertTrue("sys[1].x[0]" in un_dup.state_index) + self.assertTrue("copy of namedsys.x0" in un_dup.state_index) + self.assertTrue("sys[1].x[0]" in un_hierarchical.state_index) + self.assertTrue("sys[2].sys[1].x[0]" in un_hierarchical.state_index) + self.assertTrue("sys[1].x[0]" in u_neg.state_index) + + # Same system conflict + with warnings.catch_warnings(record=True) as warnval: + same_name_series = unnamedsys * unnamedsys + self.assertEquals(len(warnval), 1) + self.assertTrue("sys[1].x[0]" in same_name_series.state_index) + self.assertTrue("copy of sys[1].x[0]" in same_name_series.state_index) + def test_named_signals_linearize_inconsistent(self): """Mare sure that providing inputs or outputs not consistent with updfcn or outfcn fail