Skip to content

Commit a3859ef

Browse files
lukasschwabaarjaneiro
lukasschwab
and
aarjaneiro
authored
Fix float multiplication bug in NewFromFloat (#16)
Co-authored-by: Aaron Janeiro Stone <aaron.stone@alpaca.markets>
1 parent 20e4a06 commit a3859ef

File tree

3 files changed

+55
-42
lines changed

3 files changed

+55
-42
lines changed

‎README.md‎

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -74,35 +74,38 @@ Generally, for general case (99%), the speedup varies from 5x to 100x.
7474
$ make bench
7575
go test -bench=. --cpuprofile profile.out --memprofile memprofile.out
7676
goos: darwin
77-
goarch: amd64
77+
goarch: arm64
7878
pkg: github.com/alpacahq/alpacadecimal
79-
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
80-
BenchmarkValue/alpacadecimal.Decimal_Cached_Case-16 314870084 3.498 ns/op
81-
BenchmarkValue/alpacadecimal.Decimal_Optimized_Case-16 15383466 70.27 ns/op
82-
BenchmarkValue/alpacadecimal.Decimal_Fallback_Case-16 5603755 209.2 ns/op
83-
BenchmarkValue/decimal.Decimal-16 6167956 184.5 ns/op
84-
BenchmarkValue/eric.Decimal-16 7021383 162.2 ns/op
85-
BenchmarkAdd/alpacadecimal.Decimal-16 556380649 2.132 ns/op
86-
BenchmarkAdd/decimal.Decimal-16 15557970 68.31 ns/op
87-
BenchmarkAdd/eric.Decimal-16 27423730 40.34 ns/op
88-
BenchmarkSub/alpacadecimal.Decimal-16 268269063 4.410 ns/op
89-
BenchmarkSub/decimal.Decimal-16 17239782 59.17 ns/op
90-
BenchmarkSub/eric.Decimal-16 24690660 40.81 ns/op
91-
BenchmarkScan/alpacadecimal.Decimal-16 87226915 13.46 ns/op
92-
BenchmarkScan/decimal.Decimal-16 6075110 191.1 ns/op
93-
BenchmarkScan/eric.Decimal-16 6422792 174.4 ns/op
94-
BenchmarkMul/alpacadecimal.Decimal-16 168732728 7.176 ns/op
95-
BenchmarkMul/decimal.Decimal-16 16051546 66.57 ns/op
96-
BenchmarkMul/eric.Decimal-16 39927952 28.20 ns/op
97-
BenchmarkDiv/alpacadecimal.Decimal-16 152054401 7.772 ns/op
98-
BenchmarkDiv/decimal.Decimal-16 4098888 281.7 ns/op
99-
BenchmarkDiv/eric.Decimal-16 34245668 31.42 ns/op
100-
BenchmarkString/alpacadecimal.Decimal-16 385985688 3.032 ns/op
101-
BenchmarkString/decimal.Decimal-16 7750777 150.9 ns/op
102-
BenchmarkString/eric.Decimal-16 6694531 167.0 ns/op
103-
BenchmarkRound/alpacadecimal.Decimal-16 88814521 11.92 ns/op
104-
BenchmarkRound/decimal.Decimal-16 4333029 255.7 ns/op
105-
BenchmarkRound/eric.Decimal-16 55717095 21.34 ns/op
79+
cpu: Apple M3
80+
BenchmarkValue/alpacadecimal.Decimal_Cached_Case-8 579633375 2.084 ns/op
81+
BenchmarkValue/alpacadecimal.Decimal_Optimized_Case-8 33500136 35.10 ns/op
82+
BenchmarkValue/alpacadecimal.Decimal_Fallback_Case-8 12971452 91.12 ns/op
83+
BenchmarkValue/decimal.Decimal-8 14983346 80.26 ns/op
84+
BenchmarkValue/eric.Decimal-8 13220779 93.22 ns/op
85+
BenchmarkAdd/alpacadecimal.Decimal-8 863144540 1.385 ns/op
86+
BenchmarkAdd/decimal.Decimal-8 34509368 35.58 ns/op
87+
BenchmarkAdd/eric.Decimal-8 69539348 17.16 ns/op
88+
BenchmarkSub/alpacadecimal.Decimal-8 501099547 2.394 ns/op
89+
BenchmarkSub/decimal.Decimal-8 40411579 28.76 ns/op
90+
BenchmarkSub/eric.Decimal-8 69800077 17.12 ns/op
91+
BenchmarkScan/alpacadecimal.Decimal-8 122420659 10.02 ns/op
92+
BenchmarkScan/decimal.Decimal-8 12091557 99.72 ns/op
93+
BenchmarkScan/eric.Decimal-8 12087218 96.06 ns/op
94+
BenchmarkMul/alpacadecimal.Decimal-8 323009985 3.722 ns/op
95+
BenchmarkMul/decimal.Decimal-8 33682357 34.52 ns/op
96+
BenchmarkMul/eric.Decimal-8 91006764 12.58 ns/op
97+
BenchmarkDiv/alpacadecimal.Decimal-8 266056830 4.517 ns/op
98+
BenchmarkDiv/decimal.Decimal-8 8536772 139.9 ns/op
99+
BenchmarkDiv/eric.Decimal-8 83571278 14.34 ns/op
100+
BenchmarkString/alpacadecimal.Decimal-8 613455636 1.958 ns/op
101+
BenchmarkString/decimal.Decimal-8 15399207 77.92 ns/op
102+
BenchmarkString/eric.Decimal-8 14207025 82.22 ns/op
103+
BenchmarkRound/alpacadecimal.Decimal-8 800181822 1.498 ns/op
104+
BenchmarkRound/decimal.Decimal-8 10937922 109.2 ns/op
105+
BenchmarkRound/eric.Decimal-8 140659539 8.512 ns/op
106+
BenchmarkNewFromDecimal/alpacadecimal.Decimal.NewFromDecimal-8 100000000 11.68 ns/op
107+
BenchmarkNewFromDecimal/alpacadecimal.Decimal.RequireFromString-8 11680768 103.5 ns/op
108+
BenchmarkNewFromDecimal/alpacadecimal.Decimal.New-8 645217516 1.865 ns/op
106109
PASS
107-
ok github.com/alpacahq/alpacadecimal 37.671s
110+
ok github.com/alpacahq/alpacadecimal 40.632s
108111
```

‎decimal.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -161,16 +161,16 @@ func NewFromBigInt(value *big.Int, exp int32) Decimal {
161161
//
162162
// NOTE: this will panic on NaN, +/-inf
163163
func NewFromFloat(f float64) Decimal {
164-
picoFloat := f * float64(scale)
165-
picoInt64 := int64(picoFloat)
166-
167-
// check if it's within range and is whole number
168-
// integer overflow is accounted for via the `picoFloat == float64(picoInt64)` check
169-
if picoInt64 >= minIntInFixed && picoInt64 <= maxIntInFixed && picoFloat == float64(picoInt64) {
170-
return Decimal{fixed: picoInt64}
164+
if math.IsNaN(f) || math.IsInf(f, 0) {
165+
return newFromDecimal(decimal.NewFromFloat(f))
171166
}
172-
173-
return newFromDecimal(decimal.NewFromFloat(f))
167+
// Convert float to string to avoid precision issues
168+
str := strconv.FormatFloat(f, 'f', -1, 64)
169+
d, err := NewFromString(str)
170+
if err != nil {
171+
return newFromDecimal(decimal.NewFromFloat(f))
172+
}
173+
return d
174174
}
175175

176176
// fallback:

‎decimal_test.go‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,20 @@ func TestDecimal(t *testing.T) {
162162
})
163163

164164
t.Run("NewFromFloat", func(t *testing.T) {
165-
x := alpacadecimal.NewFromFloat(1.234567)
166-
y, err := alpacadecimal.NewFromString("1.234567")
167-
require.NoError(t, err)
168-
shouldEqual(t, x, y)
165+
{
166+
x := alpacadecimal.NewFromFloat(1.234567)
167+
y, err := alpacadecimal.NewFromString("1.234567")
168+
require.NoError(t, err)
169+
shouldEqual(t, x, y)
170+
}
171+
{
172+
// This input caused optimized NewFromFloat to return an incorrect
173+
// value.
174+
x := alpacadecimal.NewFromFloat(17600.095)
175+
y, err := alpacadecimal.NewFromString("17600.095")
176+
require.NoError(t, err)
177+
shouldEqual(t, x, y)
178+
}
169179
})
170180

171181
t.Run("NewFromFloat32", func(t *testing.T) {

0 commit comments

Comments
 (0)