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

math/big: Float.Rat() does not return the same number as Float.String() #66209

Closed
tux21b opened this issue Mar 8, 2024 · 5 comments
Closed
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@tux21b
Copy link
Contributor

tux21b commented Mar 8, 2024

Go version

go version go1.22.0 windows/amd64

Output of go env in your module/workspace:

set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Christoph\AppData\Local\go-build
set GOENV=C:\Users\Christoph\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\Christoph\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Christoph\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:/Users/Christoph/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.0.windows-amd64
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Users\Christoph\go\pkg\mod\golang.org\toolchain@v0.0.1-go1.22.0.windows-amd64\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.22.0
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=0
set GOMOD=NUL
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-s
ections -fmessage-length=0 -ffile-prefix-map=C:\Users\CHRIST~1\AppData\Local\Temp\go-build3241685064=/tmp/go-build -gno-record-gcc-switches

What did you do?

func TestBug1(t *testing.T) {
	f, ok := new(big.Float).SetString("0.025")
	require.True(t, ok)
	require.Equal(t, "0.025", f.String())

	r, exact := f.Rat(nil)
	require.Equal(t, big.Exact, exact)
	require.Equal(t, "1/40", r.String()) // fails with: 14757395258967641293/590295810358705651712
}

func TestBug2(t *testing.T) {
	f := new(big.Float).SetPrec(53).SetMode(big.ToNearestEven).SetFloat64(0.025)
	require.Equal(t, "0.025", f.String())

	r, exact := f.Rat(nil)
	require.Equal(t, big.Exact, exact)
	require.Equal(t, "1/40", r.String()) // fails with: 3602879701896397/144115188075855872
}

playground: https://go.dev/play/p/2xDuiRHMJgn

What did you see happen?

A big.Float with the value "0.025" returns a rational number of 14757395258967641293/590295810358705651712 or 3602879701896397/144115188075855872

What did you expect to see?

I expected that a big.Float with the value of "0.025" (according to its precision and rounding mode) to have a rational number representation of "1/40".

@randall77
Copy link
Contributor

The value 0.025 is not exactly representable as a big.Float (no matter what the precision).
That's the underlying problem here. There's also the precision to which big.Float.String reports its result, which kind of hides that fact.

@mknyszek mknyszek added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 9, 2024
@mknyszek mknyszek added this to the Backlog milestone Mar 9, 2024
@mknyszek
Copy link
Contributor

mknyszek commented Mar 9, 2024

Is there anything actionable here? Based on @randall77's last comment I get the impression that the answer is no. Closing for now, but please comment if you think it should be reopened. Thanks.

@mknyszek mknyszek closed this as not planned Won't fix, can't repro, duplicate, stale Mar 9, 2024
@tux21b
Copy link
Contributor Author

tux21b commented Mar 9, 2024

I know that a value of 0.0025 can not be represented exactly as floating point number, but according to the documentation, a big.Float has a precision and a rounding mode, therefore I expected methods of big.Float (like Rat) to take the precision and rounding mode into account.

There is no documentation that says that Rat() behaves differently and ignores the rounding mode and precision. I think that's a reasonable assumption and I think that at least a documentation change would be good, however I would prefer full support for precision and rounding mode in Rat().

@randall77
Copy link
Contributor

randall77 commented Mar 9, 2024

No rounding or precision is required for big.Float -> big.Rat. Every big.Float is exactly representable as a big.Rat. (*big.Float).Rat's documentation says the result is Exact if the input is not infinity.

Are you saying we should do the same-ish thing String does by finding the shortest base-10 representation that evaluates to the same big.Float (given the big.Float's precision) and converting that to a big.Rat?

@tux21b
Copy link
Contributor Author

tux21b commented Mar 9, 2024

Good point. Both views sound reasonable to me and maybe both variants make sense. I've read the documentation (including the "exact" part), but didn't realized that it meant that the precision and rounding mode (both of which are essential properties of a big.Float in my view) would be ignored. But that's my mistake.

Currently, there is no easy way to parse a float64 value (which might already contain "rounding errors" because of the floating point representation as well as the limited precision of 53 bits) into a big.Rat. We use big.Rat internally but lots of inputs are float64.

  • new(big.Rat).SetFloat64(0.025) does not work. It initializes the rationale number with the exact value from the floating point representation and not with the value the user initially wanted.
  • new(big.Rat).SetString("0.025000000000000001") does not know that the input is a float64 with a limited precision of 53 bits and also stores the "exact" result.
  • new(big.Rat).SetString("0.025") would work of course, but a lot of external applications (including Excel for example) store float64s like the example above. The only solution I have found so far is to use strconv.ParseFloat -> strconv.FormatFloat -> (*big.Rat).SetString which looks very ineffizient and cumbersome. And even if you already have a float64, calling (*big.Rat).SetFloat64 would be wrong (a common and easy to make error in my opinion). Formatting and re-parsing is necessary. Code like that seems to be quite common across Go packages.

It might be better to create a proposal at this point, but I think its currently very hard to initialize big.Rat with a float64 as "expected by the user". Maybe something like (*big.Rat).SetFloat64(0.025).RoundWithPrec(53, big.ToNearestEven) would be helpful. I was initially hoping to work around the problem by parsing into a big.Float first.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

4 participants