Repository navigation
NonlinearIOSystem.__str__() should not fail for successfully constructed systems #329
Description
The __str__ method in InputOutputSystem assumes that the ninputs, noutputs and nstates have all been set to integers, but the constructors do not guarantee this. For example:
>> sys = ctl.NonlinearIOSystem(lambda t, x, u, params: x)
>> print(sys)
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
<ipython-input-4-85c64df40808> in <module>()
----> 1 print(sys)
/home/arnold/pythonBox/control_dev/python-control-rabraker/control/iosys.py in __str__(self)
214 """String representation of an input/output system"""
215 str = "System: " + (self.name if self.name else "(none)") + "\n"
--> 216 str += "Inputs (%d): " % self.ninputs
217 for key in self.input_index: str += key + ", "
218 str += "\nOutputs (%d): " % self.noutputs
TypeError: %d format: a number is required, not NoneType
I would think that once a system is constructed, __str__() should always succeed. The simplest way to fix this is to replace %d with %s in the format string, so it can accept NoneType. I can submit a PR and add a regression test if this is the desired fix.
This behavior also means that when noutputs etc are not set, many of the overload operators give unhelpful errors
>>> sys = ctl.NonlinearIOSystem(lambda t, x, u, params: x)
>>> sys2 = sys * sys
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
<ipython-input-27-ea1505c3dfb9> in <module>()
----> 1 sys2 = sys * sys
/home/arnold/pythonBox/control_dev/python-control-rabraker/control/iosys.py in __mul__(sys2, sys1)
252
253 # Return the series interconnection between the systems
--> 254 newsys = InterconnectedSystem((sys1, sys2))
255
256 # Set up the connecton map
/home/arnold/pythonBox/control_dev/python-control-rabraker/control/iosys.py in __init__(self, syslist, connections, inplist, outlist, inputs, outputs, states, params, dt, name)
949 raise TypeError("System '%s' must define number of inputs, "
950 "outputs, states in order to be connected" %
--> 951 sys)
952
953 # Keep track of the offsets into the states, inputs, outputs
/home/arnold/pythonBox/control_dev/python-control-rabraker/control/iosys.py in __str__(self)
214 """String representation of an input/output system"""
215 str = "System: " + (self.name if self.name else "(none)") + "\n"
--> 216 str += "Inputs (%d): " % self.ninputs
217 for key in self.input_index: str += key + ", "
218 str += "\nOutputs (%d): " % self.noutputs
TypeError: %d format: a number is required, not NoneType
As something of a side note/question/point for discussion: I'm having a hard time groking why ninputs, noutputs and nstates are allowed to be None. Is there a downside to making inputs etc a required rather than optional argument in the NonlinearIOSystem constructor? It seems that might make the class easier to use because then all the operators (__mul__() etc) would always work and not depend on how the user called the constructor.
Activity
murrayrm commentedon Jul 22, 2019
Completely agree that these should not be failing. Will work on a fix.
The original reason for allowing ninputs, noutputs, nstates to be None was to allow users to just pass the functions defining an input/output system and then have the various member functions "figure out" the dimensions when needed. So, for example, if you linearize a system about an operating point
x0, u0then you can tell from the dimensions ofx0andu0how many states and inputs you have. Then when you call the output function, you can figure out how many outputs you have.Not sure if that functionality is going to be that useful, but at the least there should not be errors of the sort you are pointing out above.
murrayrm commentedon Dec 30, 2019
Fixed in PR #355