Skip to content

Fixed InterconnectedSystems name bugs. #400

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.

Sign up for GitHub

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

Merged
murrayrm merged 16 commits into from
Oct 17, 2020
Merged

Fixed InterconnectedSystems name bugs. #400

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
672f229
Made sure interconnectedsystems keeps names of parts.
samlaf May 14, 2020
15a442a
Update control/iosys.py
samlaf May 18, 2020
77f9441
Update control/iosys.py
samlaf May 18, 2020
d45a6c0
Fixed signal names for InterconnectedSystems.
samlaf May 20, 2020
b688729
Merge branch 'interconnectedsystems' of https://github.com/samlaf/pyt…
samlaf May 20, 2020
c8fba97
Update control/iosys.py
samlaf Jul 15, 2020
bd38dea
Update control/iosys.py
samlaf Jul 15, 2020
35676f2
Fixed some naming convention problems and added unit tests for naming…
samlaf Jul 16, 2020
90705d4
Merge branch 'interconnectedsystems' of https://github.com/samlaf/pyt…
samlaf Jul 16, 2020
69cc8c5
Removed for loops from tests to get better error messages
samlaf Jul 19, 2020
e76a4f8
Update control/iosys.py
samlaf Jul 19, 2020
abb1f19
Changed a lot of naming convention code/unit test following pull-requ…
samlaf Jul 19, 2020
f955b3a
Merge branch 'interconnectedsystems' of https://github.com/samlaf/pyt…
samlaf Jul 19, 2020
3ec2f02
Fixed system naming conventions and unit tests. Still need to do sign…
samlaf Jul 19, 2020
8c957b2
Merge branch 'master' into interconnectedsystems
murrayrm Jul 20, 2020
b2c8d44
Finished test_signals_naming_convention unit test
samlaf Jul 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 43 additions & 17 deletions control/iosys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <sys[id]> is generated with a unique integer id.

Attributes
----------
Expand Down Expand Up @@ -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)
Comment thread
samlaf marked this conversation as resolved.
Resolved
return newsys


Expand Down Expand Up @@ -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 <sys[id]> is generated with a unique integer id.

Returns
-------
Expand Down Expand Up @@ -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 <sys[id]> is generated with a unique integer id.

Returns
-------
Expand Down Expand Up @@ -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 '<subsys_name>.<state_name>',
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
Expand All @@ -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 <sys[id]> is generated with a unique integer id.

"""
# Convert input and output names to lists if they aren't already
Expand Down Expand Up @@ -904,24 +925,29 @@ def __init__(self, syslist, connections=[], inplist=[], outlist=[],
system_count += 1
Comment thread
samlaf marked this conversation as resolved.
Resolved

# 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()]

@samlaf samlaf Jul 15, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@murrayrm This is the part that would break if you add a named signal with a generic 'sys[i]' that will cause a conflict (it's no different from having two systems with the same name). The only "bad" thing that happens is that the system will be missing the state names of the systems with conflicting names (only the first 'sys[i].states' will be present).
Right now we are only raising a warning. As I mention in the comment, users can use sys.copy() first to create a unique name. Is this ok?

Edit: see the comment above this snippet of code... I'm new to this and not sure how to include it in the comment.

# Create the I/O system
super(InterconnectedSystem, self).__init__(
Expand Down
62 changes: 58 additions & 4 deletions control/tests/iosys_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
bnavigator marked this conversation as resolved.
Resolved

# 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)
Comment thread
samlaf marked this conversation as resolved.
Resolved

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
Comment thread
samlaf marked this conversation as resolved.
Resolved
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"""
Expand Down