ilayn / harold

An open-source systems and controls toolbox for Python3
MIT License
173 stars 19 forks source link

bug in simulate_impulse_response: no "ts" if "t" is input #45

Closed twmacro closed 5 years ago

twmacro commented 5 years ago

Hi! This is a simple little bug. If you call simulate_impulse_response with an argument for the time vector, the internal variable "ts" never gets created. Then, this line fails:

u[0] = 1./ts

For fun, here is a complete example that mimics my real problem:

import numpy as np
import matplotlib.pyplot as plt
import harold

dt = 1.0 / 50.0

# define 10 Hz filter with 0.7 damping:
w = 2 * np.pi * 10.0
num = w**2
den = [1.0, 2*0.7*w, w**2]
filt = harold.Transfer(num, den)
ss = harold.transfer_to_state(filt)
ssd = harold.discretize(ss, dt, 'foh')

t = np.arange(0., 1., dt)
y, t_ = harold.simulate_impulse_response(ssd, t)
# y, t_ = harold.simulate_impulse_response(ssd)
plt.plot(t_, y)
ilayn commented 5 years ago

Ouch, that's stupid of me. I should have tested better. Sorry about that. I'll fix it whenever I have some time in a few days. Thanks @twmacro !

ilayn commented 5 years ago

In the meantime you can directly get the impulse response with harold.impulse_response_plot(ssd) to get the plot directly. But same bug also affects that too :(

twmacro commented 5 years ago

@ilayn: Fabulous! :-) And thanks for the tip about harold.impulse_response_plot ... I'm just learning about your excellent package and hadn't noticed that yet.

ilayn commented 5 years ago

@twmacro the pleasure is mine. Please feel free to fire about my brain freezes or if you are missing a feature. It is really valuable for me to have feedback.

twmacro commented 5 years ago

@ilayn, Awesome! :-)