Skip to content
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

Vararg version of hypot does overflow and underflow on master #27141

Closed
giordano opened this issue May 17, 2018 · 5 comments · Fixed by #38158
Closed

Vararg version of hypot does overflow and underflow on master #27141

giordano opened this issue May 17, 2018 · 5 comments · Fixed by #38158
Labels
maths Mathematical functions regression Regression in behavior compared to a previous version

Comments

@giordano
Copy link
Contributor

giordano commented May 17, 2018

The point of the hypot function is to avoid underflow and overflow, however the implementation of the vararg method introduced with PR #25571 defeats this purpose:

julia> hypot(1e200, 0, 0)
Inf

julia> hypot(1e-300, 0, 0)
0.0

Instead in Julia 0.6 it gives the correct result:

ulia> hypot(1e200, 0, 0)
1.0e200

julia> hypot(1e-300, 0, 0)
1.0e-300

Edit: I've just seen this was already pointed out in #25571 (comment)

@fredrikekre
Copy link
Member

img

@stevengj
Copy link
Member

Maybe just define:

hypot(x::Number...) = vecnorm(x)

@simonbyrne
Copy link
Contributor

@stevengj I think the problem is that vecnorm is part of LinAlg.

One alternative would be

hypot(x,y,z...) = hypot(hypot(x,y), z...)

@stevengj
Copy link
Member

That would be a performance trap, in that it would be much slower than expected.

We can also just reimplement the relevant part of vecnorm: it doesn’t take more than a few lines to do the right thing.

@JeffBezanson JeffBezanson added regression Regression in behavior compared to a previous version maths Mathematical functions labels May 22, 2018
@stevengj
Copy link
Member

stevengj commented Dec 13, 2018

The following cases give even worse failures due to integer wraparound:

julia> i = typemax(Int)
9223372036854775807

julia> hypot(i,i,i)  # the result is √3 !!
1.7320508075688772

julia> i*√3
1.5975348984942514e19

julia> hypot(10^17, 10^17, 10^17)  # 3*abs2(10^17) is negative!
ERROR: DomainError with -6.437707461859213e18:

There should definitely be a test for the i = typemax(Int) case in any solution PR.

vtjnash pushed a commit to vtjnash/julia that referenced this issue Oct 23, 2020
Fixes JuliaLang#27141: The previous code led to under/overflow.
vtjnash pushed a commit to vtjnash/julia that referenced this issue Oct 26, 2020
Fixes JuliaLang#27141: The previous code led to under/overflow.
vtjnash added a commit that referenced this issue Oct 30, 2020
Fixes #27141: The previous code led to under/overflow.

Co-authored-by: Jorge Fernandez-de-Cossio-Diaz <j.cossio.diaz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maths Mathematical functions regression Regression in behavior compared to a previous version
Projects
None yet
5 participants