-
Notifications
You must be signed in to change notification settings - Fork 438
improved default time vector for time response functions that takes zeros into account. #454
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
Conversation
Thank you very much for this. It's all good. Feel free to butcher as you see fit and excuses in advance if I made mistakes here and there. |
d1da3e1
to
afdc5e3
Compare
Sorry, the merge of #441 created a merge conflict mainly due to whitespace changes. I rebased to current master for you. To continue working, if you don't have local changes and your personal fork is the remote git reset --hard origin/better-tfinal If you don't like my interference, to revert my rebase, just |
removed unicode in code to make python 2 happy Co-authored-by: Ben Greiner <code@bnavigator.de>
@bnavigator all looks good. I double checked and new code works fine with z-plane poles at zero or 1, so #441 is no longer needed, except for if it adds a unit test
|
|
I'll give up and do that if this last commit fails too. |
phew... 😅 |
@ilayn Related: your code uses is_step only for computing cont-time
responses, but not discrete-time. Is that intentional? S
On Wed, Aug 19, 2020 at 2:32 PM Ilhan Polat ***@***.***> wrote:
I double checked and new code works fine with z-plane poles at zero or 1,
phew... 😅
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#454 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AN74SSNLVA42RHGYN4ZT4QTSBRABVANCNFSM4QC4FPIA>
.
--
Sawyer Fuller
Assistant Professor of Mechanical Engineering
Adjunct, Paul G Allen School of Computer Science
University of Washington
http://faculty.washington.edu/~minster/
(Typed with my thumbs)
|
Yes, because in discrete time, we don't really need to have to get the fast dynamics contributions since they are bounded by the sampling period. I might be wrong though, didn't give too much of a careful thought. Probably can be tested with very fast poles and very slow poles. |
@ilayn thanks, makes sense. Update on the above code: whoops, I didn't actually put a pole at the origin. updated code still works:
|
Spelling error in docstring Co-authored-by: Naman Gera <namangera15@gmail.com>
@@ -65,12 +65,18 @@ | |||
capability and better automatic time vector creation | |||
Date: June 2020 | |||
|
|||
Modified by Ilhan Polat to improve automatic time vector creation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should get the credit here as far as I am concerned. I really didn't do anything other than geeking out some years ago.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Geeking out = valuable! : ) If you're ok with it, I would just say leave it in - I took credit elsewhere in the code for merging things together.
Improves the calculation of the default time vector used in
timeresp
functions, e.g.step_response
. Inspired by problems that appeared in #440 with pole-zero cancellations confusing the step response default time vector computation.I combined the logic & glue from #420 with Ilayn's code from here that does a more thorough job computing default trinal and dt: https://github.com/ilayn/harold/blob/master/harold/_time_domain.py#L280-L454
@ilayn please let me know if you're not ok with having your code pasted in like this (I noted in the code that it is your work), or if licensing information needs to be added, thanks.