-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
add hypot
fallback for 2 Numbers of the same type
#34316
Conversation
Seems like this is causing some subtyping crash when creating the sysimage? |
base/math.jl
Outdated
@@ -613,6 +613,7 @@ julia> hypot(3, 4im) | |||
hypot(x::Number, y::Number) = hypot(promote(x, y)...) | |||
hypot(x::Complex, y::Complex) = hypot(abs(x), abs(y)) | |||
hypot(x::T, y::T) where {T<:Real} = hypot(float(x), float(y)) | |||
hypot(x::T, y::T) where {T<:Number} = sqrt(abs2(x) + abs2(y)) |
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.
Doesn't this overflow/underflow, which hypot
should not do?
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.
At least use something like abs(x) * sqrt (1 + (y/x)^2)
.
Probably not worth holding up the PR. |
Why not just revert that PR and keep the old method for non-AbstractFloats? |
There already is a method for two Reals and two Complexes. So this would only apply to non-Real, non-Complex Numbers, so potentially somewhat exotic types that might not support all operations. |
59ce7c4
to
56fc981
Compare
Fix hypot with both arguments 0 Remove hypot from Furlongs in order to test the defined fallback
(cherry picked from commit ade4713)
Fix hypot with both arguments 0 Remove hypot from Furlongs in order to test the defined fallback (cherry picked from commit 8e3aa66)
#33224 broke
hypot
with Unitful.jl. There used to be a method for this. The weird thing is, in the current state it works if you pass 3 arguments tohypot
but not 2, so we should probably add this.