internal/number: change rounding again
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>
diff --git a/internal/number/decimal.go b/internal/number/decimal.go
index 62074e7..9b4035e 100644
--- a/internal/number/decimal.go
+++ b/internal/number/decimal.go
@@ -406,23 +406,35 @@
// By default we get the exact decimal representation.
verb := byte('g')
prec := -1
- // Determine rounding, if possible. As the strconv API does not return the
- // rounding accuracy (exact/rounded up|down), we can only round using
- // ToNearestEven.
- // Something like this would work:
- // AppendDigits(dst []byte, x float64, base, size, prec int) (digits []byte, exp, accuracy int)
- //
- // TODO: At this point strconv's rounding is imprecise to the point that it
- // is not useable for this purpose.
- // See https://github.com/golang/go/issues/21714
- // if r.Mode == ToNearestEven {
- // if n := r.RoundSignificantDigits(); n >= 0 {
- // prec = n
- // } else if n = r.RoundFractionDigits(); n >= 0 {
- // prec = n
- // verb = 'f'
- // }
- // }
+ // As the strconv API does not return the rounding accuracy, we can only
+ // round using ToNearestEven.
+ if r.Mode == ToNearestEven {
+ if n := r.RoundSignificantDigits(); n >= 0 {
+ prec = n
+ } else if n = r.RoundFractionDigits(); n >= 0 {
+ prec = n
+ verb = 'f'
+ }
+ } else {
+ // TODO: At this point strconv's rounding is imprecise to the point that
+ // it is not useable for this purpose.
+ // See https://github.com/golang/go/issues/21714
+ // If rounding is requested, we ask for a large number of digits and
+ // round from there to simulate rounding only once.
+ // Ideally we would have strconv export an AppendDigits that would take
+ // a rounding mode and/or return an accuracy. Something like this would
+ // work:
+ // AppendDigits(dst []byte, x float64, base, size, prec int) (digits []byte, exp, accuracy int)
+ hasPrec := r.RoundSignificantDigits() >= 0
+ hasScale := r.RoundFractionDigits() >= 0
+ if hasPrec || hasScale {
+ // prec is the number of mantissa bits plus some extra for safety.
+ // We need at least the number of mantissa bits as decimals to
+ // accurately represent the floating point without rounding, as each
+ // bit requires one more decimal to represent: 0.5, 0.25, 0.125, ...
+ prec = 60
+ }
+ }
b := strconv.AppendFloat(d.Digits[:0], abs, verb, prec, size)
i := 0
diff --git a/internal/number/decimal_test.go b/internal/number/decimal_test.go
index 04aa8b2..97c7e25 100644
--- a/internal/number/decimal_test.go
+++ b/internal/number/decimal_test.go
@@ -256,12 +256,10 @@
rc RoundingContext
out string
}{
- // TODO: uncommented tests can be restored when convert does its own
- // rounding.
- // {-0.001, scale2, "-0.00"}, // not normalized
- // {0.1234, prec3, "0.123"},
- // {1234.0, prec3, "1230"},
- // {1.2345e10, prec3, "12300000000"},
+ {-0.001, scale2, "-0.00"},
+ {0.1234, prec3, "0.123"},
+ {1234.0, prec3, "1230"},
+ {1.2345e10, prec3, "12300000000"},
{int8(-34), scale2, "-34"},
{int16(-234), scale2, "-234"},
@@ -273,18 +271,37 @@
{uint32(234), scale2, "234"},
{uint64(234), scale2, "234"},
{uint(234), scale2, "234"},
- {-1e9, scale2, "-1000000000"},
- {0.234, scale2away, "0.234"}, // rounding postponed as not ToNearestEven
+ {-1e9, scale2, "-1000000000.00"},
+ // The following two causes this result to have a lot of digits:
+ // 1) 0.234 cannot be accurately represented as a float64, and
+ // 2) as strconv does not support the rounding AwayFromZero, Convert
+ // leaves the rounding to caller.
+ {0.234, scale2away,
+ "0.2340000000000000135447209004269097931683063507080078125"},
+ {0.0249, inc0_05, "0.00"},
+ {0.025, inc0_05, "0.00"},
+ {0.0251, inc0_05, "0.05"},
{0.03, inc0_05, "0.05"},
- {0.025, inc0_05, "0"},
- {0.075, inc0_05, "0.1"},
+ {0.049, inc0_05, "0.05"},
+ {0.05, inc0_05, "0.05"},
+ {0.051, inc0_05, "0.05"},
+ {0.0749, inc0_05, "0.05"},
+ {0.075, inc0_05, "0.10"},
+ {0.0751, inc0_05, "0.10"},
+ {324, inc50, "300"},
{325, inc50, "300"},
+ {326, inc50, "350"},
+ {349, inc50, "350"},
+ {350, inc50, "350"},
+ {351, inc50, "350"},
+ {374, inc50, "350"},
{375, inc50, "400"},
+ {376, inc50, "400"},
// Here the scale is 2, but the digits get shifted left. As we use
// AppendFloat to do the rounding an exta 0 gets added.
- {0.123, roundShift, "0.123"},
+ {0.123, roundShift, "0.1230"},
{converter(3), scale2, "100"},
diff --git a/number/number_test.go b/number/number_test.go
index 96b1acb..3dcac36 100644
--- a/number/number_test.go
+++ b/number/number_test.go
@@ -64,8 +64,8 @@
want: "0.12",
}, {
desc: "max fraction overflow",
- f: Decimal(0.123, MaxFractionDigits(1e6)),
- want: "0.123",
+ f: Decimal(0.125, MaxFractionDigits(1e6)),
+ want: "0.125",
}, {
desc: "min integer overflow",
f: Decimal(0, MinIntegerDigits(1e6)),
@@ -172,7 +172,7 @@
want: "123.45‰",
}, {
desc: "percent fraction",
- f: PerMille(0.12345, Scale(1)),
+ f: PerMille(0.12344, Scale(1)),
want: "123.4‰",
}}
for _, tc := range testCases {
