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

hypot with complex arguments #31941

Closed
giordano opened this issue May 6, 2019 · 11 comments · Fixed by #31947
Closed

hypot with complex arguments #31941

giordano opened this issue May 6, 2019 · 11 comments · Fixed by #31947
Labels
complex Complex numbers docs This change adds or pertains to documentation

Comments

@giordano
Copy link
Contributor

giordano commented May 6, 2019

While looking at #31922 I noted that currently

julia> hypot(im)
1.0

julia> hypot(im, 1)
1.4142135623730951

which doesn't look correct given the meaning of hypot as "more precise version of \sqrt{\sum x_i^2}". I couldn't find any test about hypot with complex arguments, so I think this case was never taken into account.

@thchr
Copy link
Contributor

thchr commented May 6, 2019

It doesn't seem like one would ever want hypot(::Complex,::Complex) to return a complex number though: the utility of hypot is in computing lengths.
Maybe the doc-string should simply be fixed to sqrt{|x|^2+|y|^2} and sqrt{\sum |x_i|^2} for the two- and varargs method.

For what it's worth, this is what Matlab does. Numpy doesn't seem to allow complex input for hypot, which is also an option, I suppose; but technically breaking.

@giordano
Copy link
Contributor Author

giordano commented May 6, 2019

I was also thinking about disallowing complex input. The signature could be restricted from Number to Real, but yes, it's breaking

@StefanKarpinski
Copy link
Member

Arguably, the hypotenuse of a triangle with imaginary side lengths is imaginary.

@JeffBezanson JeffBezanson added the complex Complex numbers label May 6, 2019
@KlausC
Copy link
Contributor

KlausC commented May 6, 2019

An imaginary side length of a triangle is beyond my imagination. I would interpret hypot(r1+i1*im, r2+i2*im) as the length of the 4-dimensional vector (r1, i1, r2, i2), which is the equal to sqrt(abs(r1+i1*im)^2 + abs(r2+i2*im)^2).

@giordano
Copy link
Contributor Author

giordano commented May 6, 2019

@KlausC so you'd say that current behaviour is correct (and only docstrings would need to be adjusted)?

@KlausC
Copy link
Contributor

KlausC commented May 6, 2019

right, I support @thchr. Also the 1-argument version hypot(3+4im) == hypot(3, 4) makes geometric sense, if you view a complex number as a right triangle. So no objections against complex arguments and everything for a real result and geomteric interpretation.

@StefanKarpinski
Copy link
Member

I imagine (see what I did there?) that @stevengj or @simonbyrne might have a useful opinions on this.

@simonbyrne
Copy link
Contributor

One can view hypot as the operator whose reduction gives the l2-norm, which is consistent with what @thchr and @KlausC are proposing, so it would be "more precise version of sqrt(abs2(x) + abs2(y)).

@giordano
Copy link
Contributor Author

giordano commented May 6, 2019

Indeed, before PR #25571 the vararg method of hypot was using exactly (vec)norm. Now it has been replaced by sqrt(sum(abs2(y) for y in x)), which however does overflow/underflow (#27141)

@stevengj
Copy link
Member

stevengj commented May 6, 2019

Although it's possible to restrict the hypot function to real arguments, I think the extension to hypot(a,b) = norm([a,b], 2) = sqrt(abs2(a) + abs2(b)) is perfectly reasonable, and I would argue that it is the only useful extension of this function to other number types — you compute norms of complex numbers all the time, but the unconjugated sum of squares is rarely important. Since it costs us virtually nothing in code size to support the general definition, I think we should keep it. (It's what the Matlab hypot function does, for what it's worth.)

However, the documentation should be fixed to state \sqrt{|x|^2+|y|^2} instead of \sqrt{x^2+y^2}.

@stevengj stevengj added the docs This change adds or pertains to documentation label May 6, 2019
@giordano
Copy link
Contributor Author

giordano commented May 6, 2019

Ok, there seems to be consensus to keep current behaviour and fix the docstring. I've opened PR #31947.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complex Complex numbers docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants