Skip to content

Update docstring for impulse for discrete sys #812

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

Merged
merged 1 commit into from
Dec 13, 2022
Merged

Conversation

sawyerbfuller
Copy link
Contributor

Update impulse_response doc string to indicate that the impulse size is unit area for discrete systems.

Update impulse_response doc string to indicate that the impulse size is unit area for discrete systems.
@murrayrm
Copy link
Member

murrayrm commented Dec 11, 2022

We definitely need to update the documentation, but I wonder whether we shouldn't also consider what the right definition of the impulse response is for a discrete time system with non-unit sampling time. MATLAB apparently defines the impulse response for a discrete time system as being the response to an input that is 1 at the initial time and then 0 after that. This matches the description in the Wikipedia entry for impulse response.

@sawyerbfuller
Copy link
Contributor Author

I made this change a little while back to match what matlab does, which is a unit-area impulse for discrete-time systems: https://www.mathworks.com/help/control/ref/lti.impulse.html. This arrangement is probably what you want if you are viewing a discrete-time system as a discretized model of a continuous-time system.

Probably unit area is favored by engineering, unit magnitude by math.

Probably a subjective choice. Anybody here have further insight into such a choice?

@ilayn
Copy link

ilayn commented Dec 12, 2022

As the sampling frequency period grows the response starts to get larger if impulse is kept at unity. Hence the same dynamical system sampled at different frequencies will give different results with the same unity impulse which is not correct. In a hand-waving argument we have to scale the energy of the impulse with frequency to have equivalent supply. Otherwise, we need to type math here which is not fun :)

You can test it yourself by generating a [1, 0, 0, 0, ...] array and supply to lsim for different sampled versions of the same system to see the effect I am attempting to describe above.

@murrayrm
Copy link
Member

@ilayn's comment makes it clear why scaling by the sampling time is the right approach (and I was mistaken about what MATLAB does).

@murrayrm murrayrm merged commit 900d56e into main Dec 13, 2022
@murrayrm murrayrm added this to the 0.9.3 milestone Dec 24, 2022
@murrayrm murrayrm deleted the sawyerbfuller-patch-2 branch June 10, 2023 05:29
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.

Scaling difference in impulse response size compared to scipy.signal
3 participants