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

Jump to bottom

strconv: incorrect rounding to nearest even #21714

Closed
griesemer opened this issue Aug 31, 2017 · 5 comments
Closed

strconv: incorrect rounding to nearest even #21714

griesemer opened this issue Aug 31, 2017 · 5 comments
Labels
Documentation FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Go1.10

Comments

@griesemer
Copy link
Contributor

griesemer commented Aug 31, 2017 •
edited

The following statements

fmt.Printf("x = %.1f\n", 0.35)
fmt.Printf("x = %.1f\n", 0.45)
fmt.Printf("x = %.1f\n", 0.55)
fmt.Printf("x = %.1f\n", 0.65)

produce

x = 0.3
x = 0.5
x = 0.6
x = 0.7

(https://play.golang.org/p/kLC7Ap0cpP).

However, assuming rounding to nearest even, one would expect the result to be

x = 0.4
x = 0.4
x = 0.6
x = 0.6

(The examples use fmt.Printf but the culprit is in the underlying strconv code. The documentation doesn't explicitly state that it rounds to nearest even, but that is definitively the intent of the implementation which contains code and comments to that effect.).

The problem of course is that these numbers cannot be accurately represented in binary form and when it comes to the rounding decision, what should be .5 is sometimes below or above that value.

That said, the correct result could in fact be obtained if rounding were done on the shortest decimal representation that produces the incoming number, that is if rounding where done on the output of using the "%g" format (or strconv.FormatFloat with precision -1). Unfortunately, computing that representation is much more expensive than what happens now.

We can probably not fix this. For one, it's been like this forever. Also, a corresponding C program

#include <stdio.h>

int main() {
	printf("x = %.1f\n", 0.35);
	printf("x = %.1f\n", 0.45);
	printf("x = %.1f\n", 0.55);
	printf("x = %.1f\n", 0.65);
	return 0;
}

produces the same (incorrect) result. And so does math/big.Float formatting with the same arguments (since the code mirrors the strconv code).

But we should probably document it and perhaps even provide a "correct" routine for cases where this truly matters.
@griesemer griesemer added Documentation NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Aug 31, 2017
@griesemer griesemer added this to the Go1.10 milestone Aug 31, 2017
@gopherbot
Copy link

Change https://golang.org/cl/60730 mentions this issue: internal/number: fix more rounding issues

@bcmills
Copy link
Contributor

bcmills commented Sep 1, 2017 •
edited

I don't see what's “incorrect” about it. The spec gives exactly what happens: the arbitrary-precision constant 3.5 is rounded to its default type, which is then rounded to one digit.

The only reason to expect any different would be if floating-point constants converted to an arbitrary-precision floating-point type by default instead of a fixed-precision type.

@griesemer
Copy link
Contributor Author

@bcmills This has little to do with arbitrary precision constants since we get the same results if we do everything in float64 arithmetic, starting from numbers that can be represented exactly.

But I'm wrong nevertheless: After an exact constant f (a fraction) such as 0.35 has been converted to a float64 value x (or after a float64 division of the exact values 35.0 and 100.0) there may be an error between x and f if f cannot be represented accurately as a float64.

In the case of f = 0.35 = 35.0/100.0, the resulting x = float64(0.35) is precisely 3152519739159347/9007199254740992, which is off by -1/45035996273704960 (= -2.2204460492503130809e-17) from the original f; i.e., it is smaller than 0.35.

At this point all we have is that float64 value x, and if we then round it to one decimal mantissa the result is correctly 0.3 and not 0.4 (because x is in fact smaller than 0.35).

The following program computes the exact errors as well as exact and strconv rounded values and it turns out that for my original example above, the strconv rounded results are less than 0.05 away from the actual float64 values which means that they are correctly rounded.

@mpvl I'm closing this as working as expected. You may need to revisit https://golang.org/cl/60730.

@mpvl
Copy link
Contributor

mpvl commented Sep 1, 2017

Will undo that CL and focus on allowing people to use their own numeric types.

gopherbot pushed a commit to golang/text that referenced this issue Sep 1, 2017
@mpvl
internal/number: fix more rounding issues
5c11b69 
Removed AppendFloat-based rounding to
not hit bug golang/go#21714.

Fixed several edge cases and implemented some
missing features that were exposed as a result
of having to fall back on our own rounding.

This also fixed some previously unnoticed bugs.

Change-Id: Id675bdc9a9099eda0b9c31e5166b510b714887b5
Reviewed-on: https://go-review.googlesource.com/60730
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Nigel Tao <nigeltao@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/61492 mentions this issue: internal/number: change rounding again

gopherbot pushed a commit to golang/text that referenced this issue Oct 5, 2017
@mpvl
internal/number: change rounding again
d82c181 
Hopefully final attempt to work around golang/go#21714.

Prevent double rounding:
If we apply rounding down the road we get a
large number of decimals. Otherwise
we ask for the smallest representation.
Change test cases to not use numbers that
will round incorrectly.

Change-Id: If5c8fbcb27a867de28caf847e9ba828718482841
Reviewed-on: https://go-review.googlesource.com/61492
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
@dominikh dominikh mentioned this issue Jan 5, 2018
Closed
@golang golang locked and limited conversation to collaborators Sep 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Go1.10
Development

No branches or pull requests
4 participants
@bcmills @mpvl @griesemer @gopherbot