Skip to content

math/cmplx: Cot(0) loops infinitely #17577

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

Closed
catalase opened this issue Oct 25, 2016 · 6 comments
Closed

math/cmplx: Cot(0) loops infinitely #17577

catalase opened this issue Oct 25, 2016 · 6 comments
Milestone

Comments

@catalase
Copy link

catalase commented Oct 25, 2016

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go1.7 (I tested on play.golang.org)

What did you do?

  1. https://play.golang.org/p/TYwrN6agmD

cmplx.Cot(0) will loop infinitely

  1. https://play.golang.org/p/b4IG3wqBm_

let f be a function in the pakcage, and let g be the inverse function of f if f is invertible. If math/cmplx has no error in its implementation, then f(g(z)) is z for a complex z that is well-defined on f and g, which I tested.

but some function calculates wrongly, and some has poor error.

@odeke-em
Copy link
Member

Just a drive by comment here, if I recall from my rusty Maths:

  • cot(x) = 1/tan(x) where tan(0) = 0
    coming from 1/tan(x) = cos(x)/sin(x):
    and cos(0) = 1, sin(0) = 0 so:
    cot(0) = 1/0 => Infinity.

I wonder if the package has a problem with detecting this case and should return infinity asap.
/cc @griesemer @rsc

@odeke-em odeke-em changed the title math/cmplx: cmplx.Cot(0) will loop infinitely, the cmplx package seems broken math/cmplx: Cot(0) loops infinitely Oct 25, 2016
@0xmohit
Copy link
Contributor

0xmohit commented Oct 25, 2016

The issue happens due to floating-point comparison here. In this specific case, t/d is NaN and the comparison returns false resulting in an infinite loop. Will send a CL.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/31952 mentions this issue.

@rakyll rakyll added this to the Go1.8 milestone Oct 25, 2016
gopherbot pushed a commit that referenced this issue Oct 25, 2016
The condition to determine if any further iterations are needed is
evaluated to false in case it encounters a NaN. Instead, flip the
condition to keep looping until the factor is greater than the machine
roundoff error.

Updates #17577

Change-Id: I058abe73fcd49d3ae4e2f7b33020437cc8f290c3
Reviewed-on: https://go-review.googlesource.com/31952
Reviewed-by: Robert Griesemer <gri@golang.org>
@odeke-em
Copy link
Member

@0xmohit and @griesemer, did y'all mean to say "Fixes" instead of "Updates" with the CL https://go-review.googlesource.com/31952 that prevents an infinite loop in tanSeries, or is there more work for this issue?

@0xmohit
Copy link
Contributor

0xmohit commented Oct 26, 2016

I probably specified "Updates" because more tests could be added for Cot. However, those aren't strictly related to this change.

@griesemer
Copy link
Contributor

@odeke-em Thanks for heads-up. Yes, I would consider this fixed.

@golang golang locked and limited conversation to collaborators Oct 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants