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
math: Sin(Pi) and Cos(Pi/2) should return 0 but don't #38039
Comments
The number π isn't exactly representable in floating point. Your code won't even work, because the equality won't be satisfied for many multiples, because of the cited rounding error. For example, this code:
prints
Floats are weird. A lot of the things you expect from real numbers do not hold for floats. |
@randall77 I do understand that floating point arithmetic is not precise. However, would it not suffice to be within ε of an integer? e.g. |
Also, somewhat of an arbitrary and very subjective point, is that I think even if we cannot solve this for all k, we should at least solve it for the primary domain of the functions [-π, π]. |
I see two problems with solving this, even as you describe. |
All very fair points. That said, unless it makes a noticeable difference in performance, I don't see why we would not add the simple special case checks. At best, it is getting the right answer and at worst it is giving the same (wrong) answer that we have at the moment. On my machine it adds about
package main_test
import (
"testing"
"math"
)
var GlobalF float64
var sin = math.Sin
var x = 1.23456789
func MathSin(x float64) float64 { return sin(x) }
func Sin(x float64) float64 {
if r := x/math.Pi; r == math.Round(r) {
return 0
}
return sin(x)
}
func BenchmarkMathSin(b *testing.B) {
for i:= 0; i < b.N; i++ {
GlobalF = MathSin(x)
}
}
func BenchmarkSin(b *testing.B) {
for i:= 0; i < b.N; i++ {
GlobalF = Sin(x)
}
} |
40% overhead |
As the title says, the Sine and Cosine functions do not return zeros as expected.
Looking at the source, these values (which are surely special cases) are not checked in tests.
Could we add special cases to Sin and Cos (and Sincos) to return 0 identically at the required arguments, i.e.
Sin(k*Pi) = Cos((2*k+1)*Pi/2) = 0
, and add explicit tests for the first cases at least?For example, a simple solution could be along the lines of:
What did you do?
https://play.golang.org/p/fqzS-dwzIFw
The text was updated successfully, but these errors were encountered: