Skip to content

Bug in step_response() #374

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

Jump to bottom
Closed
rlegnain opened this issue Feb 20, 2020 · 4 comments · Fixed by #383
Closed

Bug in step_response() #374

rlegnain opened this issue Feb 20, 2020 · 4 comments · Fixed by #383
Assignees
@murrayrm
Labels
bug

Comments

@rlegnain
Copy link

rlegnain commented Feb 20, 2020 •
edited
Loading

Hi,
I found a bug in step_response(). An error happens when use step_response() on G(s)=1/1. Here is the code:

    u = control.TransferFunction([1], [1])  # or control.tf([1], [1])
    t, y = control.step_response(u)   # error happen in this line.

When I use scipy, it works perfectly

The error message is:

File "C:\Users\rlegnain\AppData\Roaming\Python\Python35\site-packages\control\timeresp.py", line 455, in step_response
   T = _default_response_times(sys.A, 1000)
 File "C:\Users\rlegnain\AppData\Roaming\Python\Python35\site-packages\scipy\signal\ltisys.py", line 2058, in _default_response_times
   vals = linalg.eigvals(A)
 File "C:\Users\rlegnain\AppData\Roaming\Python\Python35\site-packages\scipy\linalg\decomp.py", line 769, in eigvals
   homogeneous_eigvals=homogeneous_eigvals)
 File "C:\Users\rlegnain\AppData\Roaming\Python\Python35\site-packages\scipy\linalg\decomp.py", line 233, in eig
   compute_vr=compute_vr)
 File "C:\Users\rlegnain\AppData\Roaming\Python\Python35\site-packages\scipy\linalg\lapack.py", line 617, in _compute_lwork
   "%d" % (info,))
ValueError: Internal work array size computation failed: -5
@bnavigator
Copy link
Contributor

Can reproduce (with a different Exception raised). Properly formatted (Use the "insert code" function of github when posting code:

In [1]: import control

In [2]: u = control.TransferFunction([1],[1])

In [3]: u
Out[3]: 

1
-
1


In [4]: t, y = control.step_response(u)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-4-7a899e9160fd> in <module>
----> 1 t, y = control.step_response(u)

/usr/lib/python3.7/site-packages/control/timeresp.py in step_response(sys, T, X0, input, output, transpose, return_x, squeeze)
    514     if T is None:
    515         if isctime(sys):
--> 516             T = _default_response_times(sys.A, 100)
    517         else:
    518             # For discrete time, use integers

/usr/lib64/python3.7/site-packages/scipy/signal/ltisys.py in _default_response_times(A, n)
   2056     # TODO: This could use some more work.
   2057     # For example, what is expected when the system is unstable?
-> 2058     vals = linalg.eigvals(A)
   2059     r = min(abs(real(vals)))
   2060     if r == 0.0:

/usr/lib64/python3.7/site-packages/scipy/linalg/decomp.py in eigvals(a, b, overwrite_a, check_finite, homogeneous_eigvals)
    767     return eig(a, b=b, left=0, right=0, overwrite_a=overwrite_a,
    768                check_finite=check_finite,
--> 769                homogeneous_eigvals=homogeneous_eigvals)
    770 
    771 

/usr/lib64/python3.7/site-packages/scipy/linalg/decomp.py in eig(a, b, left, right, overwrite_a, overwrite_b, check_finite, homogeneous_eigvals)
    231     lwork = _compute_lwork(geev_lwork, a1.shape[0],
    232                            compute_vl=compute_vl,
--> 233                            compute_vr=compute_vr)
    234 
    235     if geev.typecode in 'cz':

/usr/lib64/python3.7/site-packages/scipy/linalg/lapack.py in _compute_lwork(routine, *args, **kwargs)
    804     """
    805     dtype = getattr(routine, 'dtype', None)
--> 806     ret = routine(*args, **kwargs)
    807     if ret[-1] != 0:
    808         raise ValueError("Internal work array size computation failed: "

ValueError: On entry to DGEEV parameter number 5 had an illegal value

In [5]:

@bnavigator
Copy link
Contributor

sys.A is empty for a unit TransferFunction

@ilayn
Copy link

ilayn commented Feb 20, 2020

It's on my to do list to improve scipy.linalg.eig API for 1.5. 0. This shouldn't have happened regardless of the control issue and the error should have been raised from eig.

I'll add an issue on the scipy side.

@murrayrm
Copy link
Member

At the same time, step_response probably shouldn't be computing default response times for an empty matrix (or _default_response_times should handle as a special case).

@murrayrm murrayrm self-assigned this Feb 21, 2020
@murrayrm murrayrm added the bug label Feb 21, 2020
@murrayrm murrayrm mentioned this issue Mar 22, 2020
Merged
@murrayrm murrayrm linked a pull request Mar 22, 2020 that will close this issue
fix computation of default response time for constant systems #383
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@murrayrm murrayrm

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix computation of default response time for constant systems murrayrm/python-control
4 participants
@murrayrm @ilayn @bnavigator @rlegnain