Repository navigation
Default simulation times for discrete-time step, impulse and initial condition response is wrong #287
Description
Activity
Not just discrete time systems but also to get meaningful arrays, continuous time systems also need to be checked a bit more thoroughly. Main issue is that by just getting the pole structure is not sufficient to get the acceptable dt and tfinal. My goto examples
import control as cnt import matplotlib.pyplot as plt plt.plot(*cnt.step_response(cnt.tf(1, [1, 1001, 1000]))) plt.plot(*cnt.step_response(cnt.tf([1, 2, 0], [1, 1001, 1000])))
These give
and
Even though the pole structure is the same we get a completely different level of detail. First is nice but the second is not so nice. Here the issue is that only the slowest stable root is being evaluated for the tfinal and the sampling rate. We can also make the same argument for oscillating modes that certain combinations would lead to ugly results for example
plt.plot(*cnt.step_response(cnt.tf([1, -200, -500], [1, 2, 1000])))gives
I've spent quite some time on this and definitely I'm quite not finished but here is the main argument in the docstring https://github.com/ilayn/harold/blob/master/harold/_time_domain.py#L280-L454
In summary, only the modes that contribute to the response should be taken into account. I still don't know how matlab does it but for example, I couldn't figure out how to check the repeated (Jordan blocks) contribute.
Any estimate for when it might be done? It would be great when you get things working if you could contribute it to the python-control library!
Well I am kind of stuck at this point. It works relatively well but there are a few corner cases that I don't know how to proceed. I am wondering whether pushing it to SciPy is a better solution since then python-control codebase doesn't need any change.
Pushing to SciPy is the right approach, though we may still need to fix the way discrete-time simulations are handled (since the current code just calls
_default_response_times(), which assumes the eigenvalues represent continuous-time time constants).Can you include an optional argument in your function that specifies whether the matrix should be evaluated as continuous or discrete time? Perhaps
dtime=True?I pulled a request with one line about it because I thought it was just lack of resolution in some discrete systems. Now I see that the problem is a little deeper. I know that it is not the same software concepts but maybe we could look at
__sim_horizon__from Octave to elucidate that?- linked a pull request that will close this issueimproved default time vector for time response functions that takes zeros into account. #454
on Aug 19, 2020
The current method for computing the default simulation time for discrete-time simulations uses a call to
scipy.signal._default_response_times(), but this uses an eigenvalue calculation that assumes continuous time. For discrete time systems, need to figure out what the right default values are for the length of a simulation for impulse, step, and initial condition responses. See also discussion in issue #239.