Default simulation times for discrete-time step, impulse and initial condition response is wrong #287

Activity

1. ilayn commented on Apr 10, 2019

   

   ilayn

   on Apr 10, 2019

   Last edited by ilayn

   More actions

   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.
2. murrayrm commented on Apr 11, 2019

   

   murrayrm

   on Apr 11, 2019

   MemberAuthor

   More actions

   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!
3. ilayn commented on Apr 11, 2019

   

   ilayn

   on Apr 11, 2019

   More actions

   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.
4. murrayrm commented on Apr 11, 2019

   

   murrayrm

   on Apr 11, 2019

   MemberAuthor

   More actions

   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?
5. joaoantoniocardoso commented on Apr 17, 2019

   

   joaoantoniocardoso

   on Apr 17, 2019

   Last edited by joaoantoniocardoso

   Contributor

   More actions

   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?
6. murrayrm

   self-assigned this

   on Mar 26, 2020
7. sawyerbfuller

   linked a pull request that will close this issueimproved default time vector for time response functions that takes zeros into account. #454on Aug 19, 2020
8. murrayrm

   closed this as completedin #454on Oct 17, 2020