internal/number: fix rounding issues
- rounding with large scales
- correct interpretation for engineering
- default rounding is exact representation
- simplify increments
Change-Id: I6a4959974069f5e1706e396a836bd6989b93d60d
Reviewed-on: https://go-review.googlesource.com/60350
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Nigel Tao <nigeltao@golang.org>
diff --git a/internal/number/decimal.go b/internal/number/decimal.go
index 0491c97..8d80a92 100644
--- a/internal/number/decimal.go
+++ b/internal/number/decimal.go
@@ -375,6 +375,24 @@
d.NaN = true
return
}
+ // Simple case: decimal notation
+ if r.Increment > 0 {
+ scale := int(r.MaxFractionDigits)
+ mult := 1.0
+ if scale > len(scales) {
+ mult = math.Pow(10, float64(scale))
+ } else {
+ mult = scales[scale]
+ }
+ // We multiply x instead of dividing inc as it gives less rounding
+ // issues.
+ x *= mult
+ x /= float64(r.Increment)
+ x = r.Mode.roundFloat(x)
+ x *= float64(r.Increment)
+ x /= mult
+ }
+
abs := x
if x < 0 {
d.Neg = true
@@ -384,64 +402,64 @@
d.Inf = true
return
}
- // Simple case: decimal notation
- scale := r.scale()
- prec := r.precision()
- if scale > 0 || r.Increment > 0 || prec == 0 {
- if scale > len(scales) {
- x *= math.Pow(10, float64(scale))
- } else {
- x *= scales[scale]
- }
- if r.Increment > 0 {
- inc := float64(r.Increment)
- x /= float64(inc)
- x = r.Mode.roundFloat(x)
- x *= inc
- } else {
- x = r.Mode.roundFloat(x)
- }
- d.fillIntDigits(uint64(math.Abs(x)))
- d.Exp = int32(len(d.Digits) - scale)
- return
- }
- // Nasty case (for non-decimal notation).
- // Asides from being inefficient, this result is also wrong as it will
- // apply ToNearestEven rounding regardless of the user setting.
- // TODO: expose functionality in strconv so we can avoid this hack.
+ // 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: This only supports the nearest even rounding mode.
-
- if prec > 0 {
- prec--
+ if r.Mode == ToNearestEven {
+ // We can't round if limitations are placed on both the fraction and
+ // significant digits.
+ if r.MaxFractionDigits == 0 && r.MaxSignificantDigits > 0 {
+ prec = int(r.MaxSignificantDigits)
+ } else if r.isScientific() {
+ if r.MaxIntegerDigits == 1 && (r.MaxSignificantDigits == 0 ||
+ int(r.MaxFractionDigits+1) == int(r.MaxSignificantDigits)) {
+ verb = 'e'
+ prec = int(r.MaxFractionDigits)
+ prec += int(r.DigitShift)
+ }
+ } else if r.MaxFractionDigits > 0 && r.MaxSignificantDigits == 0 {
+ verb = 'f'
+ prec = int(r.MaxFractionDigits)
+ }
}
- b := strconv.AppendFloat(d.Digits, abs, 'e', prec, size)
+
+ b := strconv.AppendFloat(d.Digits[:0], abs, verb, prec, size)
i := 0
k := 0
- // No need to check i < len(b) as we always have an 'e'.
- for {
+ beforeDot := 1
+ for i < len(b) {
if c := b[i]; '0' <= c && c <= '9' {
b[k] = c - '0'
k++
- } else if c != '.' {
+ d.Exp += int32(beforeDot)
+ } else if c == '.' {
+ beforeDot = 0
+ d.Exp = int32(k)
+ } else {
break
}
i++
}
d.Digits = b[:k]
- i += len("e")
- pSign := i
- exp := 0
- for i++; i < len(b); i++ {
- exp *= 10
- exp += int(b[i] - '0')
+ if i != len(b) {
+ i += len("e")
+ pSign := i
+ exp := 0
+ for i++; i < len(b); i++ {
+ exp *= 10
+ exp += int(b[i] - '0')
+ }
+ if b[pSign] == '-' {
+ exp = -exp
+ }
+ d.Exp = int32(exp) + 1
}
- if b[pSign] == '-' {
- exp = -exp
- }
- d.Exp = int32(exp) + 1
}
func (d *Decimal) fillIntDigits(x uint64) {
diff --git a/internal/number/decimal_test.go b/internal/number/decimal_test.go
index b017b39..ecb695a 100644
--- a/internal/number/decimal_test.go
+++ b/internal/number/decimal_test.go
@@ -265,16 +265,15 @@
{uint32(234), scale2, "234"},
{uint64(234), scale2, "234"},
{uint(234), scale2, "234"},
- {-0.001, scale2, "-0"},
+ {-0.001, scale2, "-0.00"}, // not normalized
{-1e9, scale2, "-1000000000.00"},
- {0.234, scale2, "0.23"},
- {0.234, scale2away, "0.24"},
+ {0.234, scale2away, "0.234"}, // rounding postponed as not ToNearestEven
{0.1234, prec3, "0.123"},
{1234.0, prec3, "1230"},
{1.2345e10, prec3, "12300000000"},
{0.03, inc0_05, "0.05"},
- {0.025, inc0_05, "0"},
+ {0.025, inc0_05, "0.00"}, // not normalized
{0.075, inc0_05, "0.10"},
{325, inc50, "300"},
{375, inc50, "400"},
diff --git a/internal/number/format.go b/internal/number/format.go
index 44116fa..4104789 100755
--- a/internal/number/format.go
+++ b/internal/number/format.go
@@ -56,12 +56,16 @@
// given language.
func (f *Formatter) InitScientific(t language.Tag) {
f.init(t, tagToScientific)
+ f.Pattern.MinFractionDigits = 0
+ f.Pattern.MaxFractionDigits = -1
}
// InitEngineering initializes a Formatter using the default Pattern for the
// given language.
func (f *Formatter) InitEngineering(t language.Tag) {
f.init(t, tagToScientific)
+ f.Pattern.MinFractionDigits = 0
+ f.Pattern.MaxFractionDigits = -1
f.Pattern.MaxIntegerDigits = 3
f.Pattern.MinIntegerDigits = 1
}
@@ -303,13 +307,6 @@
if numInt == 0 {
numInt = 1
}
- maxSig := int(r.MaxFractionDigits) + numInt
- minSig := int(r.MinFractionDigits) + numInt
-
- if maxSig > 0 {
- // TODO: really round to zero?
- n.round(ToZero, maxSig)
- }
// If a maximum number of integers is specified, the minimum must be 1
// and the exponent is grouped by this number (e.g. for engineering)
@@ -326,10 +323,14 @@
numInt += d
}
+ if maxSig := int(r.MaxFractionDigits); maxSig >= 0 {
+ n.round(r.Mode, maxSig+numInt)
+ }
+
n.Comma = uint8(numInt)
n.End = int32(len(n.Digits))
- if n.End < int32(minSig) {
- n.End = int32(minSig)
+ if minSig := int32(r.MinFractionDigits) + int32(numInt); n.End < minSig {
+ n.End = minSig
}
return n
}
diff --git a/internal/number/format_test.go b/internal/number/format_test.go
index fc7c36d..1273818 100755
--- a/internal/number/format_test.go
+++ b/internal/number/format_test.go
@@ -289,22 +289,25 @@
pattern: "##0E00",
test: pairs{
"100": "100\u202f×\u202f10⁰⁰",
- "12345": "10\u202f×\u202f10⁰³",
- "123.456": "100\u202f×\u202f10⁰⁰",
+ "12345": "12\u202f×\u202f10⁰³",
+ "123.456": "123\u202f×\u202f10⁰⁰",
},
}, {
pattern: "##0.###E00",
test: pairs{
- "100": "100\u202f×\u202f10⁰⁰",
- "12345": "12.34\u202f×\u202f10⁰³",
- "123.456": "123.4\u202f×\u202f10⁰⁰",
+ "100": "100\u202f×\u202f10⁰⁰",
+ "12345": "12.345\u202f×\u202f10⁰³",
+ "123456": "123.456\u202f×\u202f10⁰³",
+ "123.456": "123.456\u202f×\u202f10⁰⁰",
+ "123.4567": "123.457\u202f×\u202f10⁰⁰",
},
}, {
pattern: "##0.000E00",
test: pairs{
- "100": "100.0\u202f×\u202f10⁰⁰",
- "12345": "12.34\u202f×\u202f10⁰³",
- "123.456": "123.4\u202f×\u202f10⁰⁰",
+ "100": "100.000\u202f×\u202f10⁰⁰",
+ "12345": "12.345\u202f×\u202f10⁰³",
+ "123.456": "123.456\u202f×\u202f10⁰⁰",
+ "12.3456": "12.346\u202f×\u202f10⁰⁰",
},
}, {
pattern: "@@E0",
@@ -498,7 +501,8 @@
}{
{f.InitDecimal, "123456.78", "123,456.78"},
{f.InitScientific, "123456.78", "1.23\u202f×\u202f10⁵"},
- {f.InitEngineering, "123456.78", "123\u202f×\u202f10³"},
+ {f.InitEngineering, "123456.78", "123.46\u202f×\u202f10³"},
+ {f.InitEngineering, "1234", "1.23\u202f×\u202f10³"},
{f.InitPercent, "0.1234", "12.34%"},
{f.InitPerMille, "0.1234", "123.40‰"},
diff --git a/internal/number/pattern.go b/internal/number/pattern.go
index 89c2a1c..4c3e815 100644
--- a/internal/number/pattern.go
+++ b/internal/number/pattern.go
@@ -55,11 +55,11 @@
// It contains all information needed to determine the "visible digits" as
// required by the pluralization rules.
type RoundingContext struct {
- Increment uint32 // if > 0, round to Increment * 10^-scale()
+ Increment uint32
// TODO: unify these two fields so that there is a more unambiguous meaning
// of how precision is handled.
- MaxSignificantDigits int16 // -1 is infinite precision
- MaxFractionDigits uint16
+ MaxSignificantDigits int16 // -1 is unlimited
+ MaxFractionDigits int16 // -1 is unlimited
Mode RoundingMode
@@ -88,7 +88,7 @@
// SetScale fixes the RoundingContext to a fixed number of fraction digits.
func (r *RoundingContext) SetScale(scale int) {
r.MinFractionDigits = uint8(scale)
- r.MaxFractionDigits = uint16(scale)
+ r.MaxFractionDigits = int16(scale)
}
func (r *RoundingContext) SetPrecision(prec int) {
@@ -414,7 +414,7 @@
func (p *parser) normalizeSigDigitsWithExponent() state {
p.MinIntegerDigits, p.MaxIntegerDigits = 1, 1
p.MinFractionDigits = p.MinSignificantDigits - 1
- p.MaxFractionDigits = uint16(p.MaxSignificantDigits) - 1
+ p.MaxFractionDigits = p.MaxSignificantDigits - 1
p.MinSignificantDigits, p.MaxSignificantDigits = 0, 0
return p.exponent
}
diff --git a/internal/number/pattern_test.go b/internal/number/pattern_test.go
index f551456..af08379 100644
--- a/internal/number/pattern_test.go
+++ b/internal/number/pattern_test.go
@@ -154,6 +154,17 @@
},
},
}, {
+ "##0.###E00",
+ &Pattern{
+ FormatWidth: 10,
+ RoundingContext: RoundingContext{
+ MinIntegerDigits: 1,
+ MaxIntegerDigits: 3,
+ MaxFractionDigits: 3,
+ MinExponentDigits: 2,
+ },
+ },
+}, {
"##00.0#E0",
&Pattern{
FormatWidth: 9,
diff --git a/message/fmt_test.go b/message/fmt_test.go
index b22420d..2110bb5 100755
--- a/message/fmt_test.go
+++ b/message/fmt_test.go
@@ -475,10 +475,8 @@
{"%.4b", float32(1.0), "8388608p-23"},
{"%.4b", -1.0, "-4503599627370496p-52"},
// Test correct f.intbuf boundary checks.
- // TODO: the following cases won't work because of rounding errors. We can
- // fix this if we expose the internals of strconv.
- // {"%.68f", 1.0, zeroFill("1.", 68, "")}, // TODO(bug): rounding error
- // {"%.68f", -1.0, zeroFill("-1.", 68, "")}, // TODO(bug): rounding error
+ {"%.68f", 1.0, zeroFill("1.", 68, "")},
+ {"%.68f", -1.0, zeroFill("-1.", 68, "")},
// float infinites and NaNs
{"%f", posInf, "∞"},
{"%.1f", negInf, "-∞"},
diff --git a/message/print.go b/message/print.go
index 7e4642b..07feaa2 100644
--- a/message/print.go
+++ b/message/print.go
@@ -284,7 +284,7 @@
f.MinIntegerDigits = 1
f.MaxIntegerDigits = 0
f.MinFractionDigits = uint8(minFrac)
- f.MaxFractionDigits = uint16(maxFrac)
+ f.MaxFractionDigits = int16(maxFrac)
p.setFlags(f)
f.PadRune = 0
if p.fmt.widPresent {
@@ -313,7 +313,7 @@
} else {
f.SetPrecision(maxFrac + 1)
f.MinFractionDigits = uint8(minFrac)
- f.MaxFractionDigits = uint16(maxFrac)
+ f.MaxFractionDigits = int16(maxFrac)
}
f.MinExponentDigits = 2
p.setFlags(f)
