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

bug: Float Math Around 1.0 #53297

Closed
matthinrichsen-wf opened this issue Jun 8, 2022 · 5 comments
Closed

bug: Float Math Around 1.0 #53297

matthinrichsen-wf opened this issue Jun 8, 2022 · 5 comments

Comments

@matthinrichsen-wf
Copy link

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

$ go version
go version go1.18.3 darwin/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="/Users/matthinrichsen/Library/Caches/go-build"
GOENV="/Users/matthinrichsen/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOOS="darwin"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/homebrew/Cellar/go/1.18.3/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.18.3/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.18.3"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/4x/zvfqpl3d4zx4f7jq76sy_m6r0000gs/T/go-build3493347256=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Calculated float64(1.0) % float64(0.1)

	dividend := float64(1.0)
	divisor := float64(0.1)
	log.Println(dividend - divisor*(math.Floor(dividend/divisor)))

What did you expect to see?

Should evaluate to 0

What did you see instead?

Evaluates to -5.551115123125783e-17

@randall77
Copy link
Contributor

This is how floating-point numbers work. 0.1 is not exactly representable as a floating point number, so 1-10*0.1 is not exactly zero.

@matthinrichsen-wf
Copy link
Author

I should mention - this evaluates to 0 on Intel based machines

@randall77
Copy link
Contributor

That would be differences due to FMA. See #48145 for another example.

@randall77
Copy link
Contributor

randall77 commented Jun 8, 2022

This phenomenon happens even on amd64 https://go.dev/play/p/RF9ZzmLZusw

@matthinrichsen-wf
Copy link
Author

Thanks for taking the time to look into this, I appreciate it :)

I get that floats do weird things with rounding when numbers don't exactly fit into float representation and so you get drift - that's fine and expected. Floats gon' float.

I was surprised that compiling and running on Intel vs ARM produced different numeric results (which is why I opened this issue...even tho I forgot to mention that originally in the description, my bad). I don't necessary care which one is "right", we can find all sorts of weird float combinations that should evaluate neatly but don't. But my expectation is that regardless of target architecture, the code would execute to the same result.

We have devs using both Intel and ARM for dev, so if one dev writes a test/code that uses FMADD it may end up failing a test on the other's machine. Which is less that ideal. In this particular case, we had a test which had passed for years and only recently began to fail once devs started to use ARM for dev.

FWIW I was able to get this to avoid the issue with FMADD by doing the following:

dividend := float64(1.0)
divisor := float64(0.1)
log.Println(dividend - (divisor*math.Floor(dividend/divisor)+0.0))

prashantkhoje added a commit to prashantkhoje/geo that referenced this issue Jul 27, 2022
This was exposed while testing CockroachDB.
Following tests from pkg/sql/opt/exec/execbuilder fail due to floating point precision differences caused by FMA:
- TestExecBuild/local/geospatial
- TestExecBuild/local-vec-off/geospatial
- TestExecBuild/local-spec-planning/geospatial
- TestExecBuild/fakedist/geospatial
- TestExecBuild/fakedist-vec-off/geospatial
- TestExecBuild/fakedist-metadata/geospatial
- TestExecBuild/fakedist-disk/geospatial
- TestExecBuild/fakedist-spec-planning/geospatial

With explicit casts in this patch, these failures are resolved.

References:
https://go.dev/ref/spec#Floating_point_operators
golang/go#53297
golang/go#48145
disintegration/gift#20 (comment)
@golang golang locked and limited conversation to collaborators Jun 9, 2023
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

3 participants