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.

Sign up for GitHub

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

Jump to bottom

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
Labels
arch-arm64 FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@owulveryck
Copy link
Contributor

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
@seankhliao seankhliao added arch-arm64 NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 23, 2021
@randall77
Copy link
Contributor

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
Contributor Author

@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
Copy link
Contributor Author

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

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 added a commit to gorgonia/gorgonia that referenced this issue Feb 23, 2021
@owulveryck
fix(tanhdiff): add explicit cast for tanh test
4063816 
The spec mention that the behavior can differ accross different
architecture.
See golang/go#44528 for more details
@owulveryck owulveryck mentioned this issue Feb 23, 2021
Closed
@chewxy chewxy mentioned this issue Feb 24, 2021
Merged
chewxy pushed a commit to gorgonia/gorgonia that referenced this issue Feb 25, 2021
@dcu @owulveryck
Fix arm64 (#480)
41752ab 
* 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>
@golang golang locked and limited conversation to collaborators Feb 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-arm64 FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@randall77 @gopherbot @seankhliao @owulveryck