Skip to content
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.

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

fixed bug in calculation of percent overshoot #555

Merged
merged 2 commits into from Mar 8, 2021

Conversation

sawyerbfuller
Copy link
Contributor

@sawyerbfuller sawyerbfuller commented Mar 4, 2021

Now it gives the correct result even if the transfer function is not strictly proper.

Other changes:

  • step_info now calculates overshoot with respect to dcgain rather than the final (edit: numeric) value of the (computed) step response.
  • adjusted the length of the step response to go for a little longer, to within 0.1% rather than 1% of steady-state, to improve precision of step_info (and updated unit tests)

a remark: sys.dcgain() now always returns a complex number because it evaluates at a frequency of zero (1 for DT systems) (as a result of #542?). This returns a real-valued complex number. I think there is a reasonable argument that it should be cast to a float because currently we assume system parameters are real-valued.

@sawyerbfuller sawyerbfuller linked an issue Mar 4, 2021 that may be closed by this pull request
@coveralls
Copy link

Coverage Status

Coverage remained the same at 88.861% when pulling b36d053 on sawyerbfuller:fix-stepinfo into 66d4a53 on python-control:master.

@murrayrm
Copy link
Member

murrayrm commented Mar 4, 2021

Is #337 also addressed, by chance?

@sawyerbfuller
Copy link
Contributor Author

sorry, didn't have a chance to get to that one on this PR

@bnavigator bnavigator merged commit 92de12d into python-control:master Mar 8, 2021
@bnavigator
Copy link
Contributor

LGTM

@juanodecc
Copy link
Contributor

Dear @sawyerbfuller, the solution could have problem with systems with initial condition different of zero. I want to contribute , but at the moment, I don't now how test the last version.

I maked this example, to explain:

import numpy as np
import control as ctrl
import matplotlib.pyplot as plt

A = [[0,1,0],[0,0,1],[-1,-2,-3]]
B = [[0],[0],[1]]
C = [1,0,0]
D = 0

sys=ctrl.ss(A,B,C,D)

t= np.linspace(0,20,1000)
t1,y1 =ctrl.step_response(sys,t,[-1,0,2])

plt.plot(t1,y1)
ctrl.step_info(sys)

new_overshoot = 100. * (y1.max() - sys.dcgain()) / sys.dcgain()
print('new_overshoot =', new_overshoot)

{'RiseTime': 2.616832783582626,
'SettlingTime': 9.555404558233526,
'SettlingMin': 0.8926922608222024,
'SettlingMax': 1.1446150537879327,
'Overshoot': 15.77739060863902,
'Undershoot': 0.0,
'Peak': 1.1446150537879327,
'PeakTime': 6.06629418012336,
'SteadyStateValue': 0.9886343506022361}

new_overshoot = 38.34913836051592

@bnavigator
Copy link
Contributor

But ctrl.step_info(sys) does not have any initial conditions. It does know nothing of the step_response() call.

@murrayrm murrayrm added this to the 0.9.0 milestone Mar 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

step_info overshoot error
5 participants