GeezerVison

Coverage increased (+0.2%) to 90.079% when pulling 136d6f4 on murrayrm:timeresponse_class into 18976c1 on python-control:master.

If you call me a skeptic about object-oriented software I'll answer "guilty as charged" but this seems pretty cool and would work well with #645. I also like the potential utility for future system identification routines, and for future plotting functionality. Presumably when the latter comes we can do things like add label text, which can come in handy.

It would be nice, in the interest of minimizing friction for quick analyses of SISO systems, to have the shorter t, y attributes be the ones that are auto-squeezed, but if there is already a convention established in OptimalControlResult then maybe we should stick with that.

I'm not sure I understand the concept of a trace. is that different than the 3D array that encodes [output, input, time]?

The latest push fixes an inconsistency in the way that squeeze processing was handled for states versus input/outputs. In the original code, squeeze processing was never applied to the state vector. The consequence was that if you used squeeze=True or squeeze=False then you got a different action on the state vector than the output vector.

To avoid breaking code, I set things up so that if you access the state vector through the old interface (by assigning one of the time response systems to a tuple with return_x=True) then you get the old behavior. But if you use the new interface (response.states) the squeeze processing matches what is done on the input/output vectors. Namely, setting squeeze=True will get rid of all 1 dimensional entries and setting squeeze=False will index everything by the state, trace (corresponding to the input number of step_response and impulse_response), and time (in that order), even for SISO systems. If you leave squeeze=None then the trace is removed for SISO systems but maintained for MIMO systems.

As in the original PR, all legacy code continues to run unchanged and there are new unit tests to validate the updated behavior for the new interface.

Alternatively, I would be OK making everything consistent across output, state, and input squeeze processing, but then we would break existing code. So my thought was to leave that in place and eventually deprecate it (perhaps after we have implemented all of the suggestions in #645).

I'm not sure I understand the concept of a trace. is that different than the 3D array that encodes [output, input, time]?

It's the same. I decided to use the term "trace" because in the case of step/impulse response it happens to be the case that each trace (simulation run) corresponds to choosing one of the inputs. But more generally, you could imagine having multiple input/output traces (eg, for the aforementioned system ID) and a given trace might just be come combination of inputs.

Note also that TimeResponseData includes an inputs property of dimension (ninputs, ntraces, ntimes). So you can have a different number of traces than inputs, if you like.

Presumably when the latter [plotting functionality] comes we can do things like add label text, which can come in handy.

I've added the ability to store labels in the latest commit and set things up so that the signals names for I/O systems, if present, are copied over as the default labels for input_output_response.

@billtubbs Would this PR help with what you were working on for #645 ? ie., should we merge this PR?

I've been using the branch with this PR as my default branch for a while and everything seems OK. Ready to be merged, I think.

@billtubbs You mentioned in a separate note that you had made progress on #645. Does this conflict with that? If not, we can probably go ahead and merge.

@billtubbs You mentioned in a separate note that you had made progress on #645. Does this conflict with that? If not, we can probably go ahead and merge.

I don't think so. I haven't looked at this in detail but I was only planning to fix the specific axes and figure problems with some of the plot functions rlocus, pzmap, bode_plot, nyquist_plot, gangof4_plot, nichols_plot. My work is probably already behind the master so will need revising anyway if there are any impacts.