-
Notifications
You must be signed in to change notification settings - Fork 113
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
Remove recursion from Base.:^
#618
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #618 +/- ##
=======================================
Coverage 97.96% 97.97%
=======================================
Files 19 19
Lines 3251 3254 +3
=======================================
+ Hits 3185 3188 +3
Misses 66 66 ☔ View full report in Codecov by Sentry. |
1d79151
to
53aeab9
Compare
81f87c4
to
dd41185
Compare
Using |
dd41185
to
f28ca8d
Compare
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.
Haven't verified the code movement in detail, but the rest LGTM. Maybe the one additional test. Apart from that, good to go from my side.
Co-authored-by: Martin Holters <[email protected]>
Another unfortunate case of arithmetic overflow, as |
src/Filters/coefficients.jl
Outdated
res = ZeroPoleGain{D}(repeat(f.z, ae), repeat(f.p, ae), f.k^ae) | ||
return e < 0 ? inv(res) : res |
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.
Would this fix the overflow in computing k
in the test?
res = ZeroPoleGain{D}(repeat(f.z, ae), repeat(f.p, ae), f.k^ae) | |
return e < 0 ? inv(res) : res | |
if e < 0 | |
return ZeroPoleGain{D}(repeat(f.p, ae), repeat(f.z, ae), inv(f.k)^ae) | |
else | |
return ZeroPoleGain{D}(repeat(f.z, ae), repeat(f.p, ae), f.k^ae) | |
end |
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.
That works too.
3ffa9da
to
0276bc6
Compare
Prevents stack overflow, in the admittedly unlikely case that
-e == e
, fore == typemin(Int8)
etc.Also moves some definitions around for readability.