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

runtime: floating point error on arm64 #44528

Closed
owulveryck opened this issue Feb 23, 2021 · 4 comments
Closed

runtime: floating point error on arm64 #44528

owulveryck opened this issue Feb 23, 2021 · 4 comments

Comments

@owulveryck
Copy link

@owulveryck owulveryck commented Feb 23, 2021

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

$ go version
go version go1.16 linux/arm64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/home/ubuntu/.cache/go-build"
GOENV="/home/ubuntu/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/ubuntu/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ubuntu/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/go/pkg/tool/linux_arm64"
GOVCS=""
GOVERSION="go1.16"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build864276053=/tmp/go-build -gno-record-gcc-switches"

What did you do?

While debugging some tests for Gorgonia, we found this strange behavior:

func TestMathTanh(t *testing.T) {
	v := 0.2228940815087735
	tanh := math.Tanh(v)
	a := 1 - tanh*tanh + 1 - 1
	b := 1 - tanh*tanh - 1 + 1
	if a != b {
		t.Fail()
	}
}

This test is passing on amd64 and is failing on arm64.

What did you expect to see?

The test should pass

What did you see instead?

The test is failing

@randall77
Copy link
Contributor

@randall77 randall77 commented Feb 23, 2021

This may be multiply-add optimizations. Try

a := 1 - float64(tanh*tanh) + 1 - 1
b := 1 - float64(tanh*tanh) - 1 + 1

and see if that changes anything.

From the spec:

An implementation may combine multiple floating-point operations into a single fused operation, possibly across statements, and produce a result that differs from the value obtained by executing and rounding the instructions individually. An explicit floating-point type conversion rounds to the precision of the target type, preventing fusion that would discard that rounding.

@owulveryck
Copy link
Author

@owulveryck owulveryck commented Feb 23, 2021

@randall77 you are right, this test passes:

func TestMathTanh(t *testing.T) {
	v := 0.2228940815087735
	tanh := math.Tanh(v)
	a := 1 - float64(tanh*tanh) + 1 - 1
	b := 1 - float64(tanh*tanh) - 1 + 1
	if a != b {
		t.Fail()
	}
}
@owulveryck owulveryck closed this Feb 23, 2021
@owulveryck
Copy link
Author

@owulveryck owulveryck commented Feb 23, 2021

Anyway, I guess that the behavior should be the same on arm64 and amd64

@owulveryck owulveryck reopened this Feb 23, 2021
@randall77
Copy link
Contributor

@randall77 randall77 commented Feb 23, 2021

We do not guarantee identical floating-point behavior across architectures, as the spec describes.
If you need that, you would need to add casts in the appropriate places.

@owulveryck owulveryck closed this Feb 23, 2021
owulveryck added a commit to gorgonia/gorgonia that referenced this issue Feb 23, 2021
The spec mention that the behavior can differ accross different
architecture.
See golang/go#44528 for more details
chewxy pushed a commit to gorgonia/gorgonia that referenced this issue Feb 25, 2021
* fix(tanhdiff): add explicit cast for tanh test

The spec mention that the behavior can differ accross different
architecture.
See golang/go#44528 for more details

* Update chewxy/math32 package to latest version

This fixes most broken tests in arm64

* Fix TanhDiff test for arm64

* Fix Barzilai Borwein solver test for arm64

* Add some explanations for the floating-point precision issues

Co-authored-by: Olivier Wulveryck <olivier.wulveryck@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants