Draft
Conversation
Redesigns the fixed-point library from single int64 (10.8 digits) to dual int64 (18.18 digits) to support 18 integer digits and 18 decimal places. This provides extended range (±999 quadrillion) and precision while maintaining acceptable performance using math/big for multiplication and division operations. Major changes: - Changed struct from single fp field to hi/lo fields (16 bytes total) - Updated all arithmetic operations with sign-consistent normalization - Implemented arbitrary precision Mul/Div using math/big - Extended precision from 8 to 18 decimal places - Updated all tests to reflect new precision expectations - Binary serialization format changed (v2.0 breaking change) Performance characteristics: - Addition: 2.4 ns/op, 0 allocations - Multiplication: 163.7 ns/op, 5 allocations - Division: 350.1 ns/op, 10 allocations - Comparison: 1.88 ns/op, 0 allocations All tests passing with extended range validation. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Owner
|
I don’t think this PR is necessary. You might as well just use bignum which uses variable size storage |
Fixes five critical bugs discovered during thorough testing: 1. Overflow detection in Add/Sub operations - Add and Sub now properly return NaN when result exceeds ±999,999,999,999,999,999 - Prevents invalid values beyond 18-digit range 2. Overflow detection in Mul operation - Multiplication now checks result against maxHi after normalization - Ensures consistency between Add and Mul for large values 3. Input validation overflow check - Changed from imprecise float comparison to direct integer check - NewS and NewI now use maxHi constant for accurate validation 4. Division rounding error with negative numbers - Fixed -1/3 returning -0.333333333333333335 instead of -0.333333333333333333 - Changed from DivMod (Euclidean) to QuoRem (truncated division) - Fixed lo calculation to use subtraction instead of Mod for sign consistency 5. String parsing for "-.5" format - Added handling for sign prefix before decimal point with no integer part - Parsing "-.5", "+.5" now works correctly Added comprehensive_test.go with 20+ edge case tests covering: - Sign consistency across operations - Overflow and underflow conditions - Rounding edge cases (floor, ceil, round) - Negative zero handling - Operation properties (commutativity, associativity, identity) - Binary serialization round-trips - Float conversion precision limits - String parsing edge cases All 50+ tests passing. Division performance improved from 350ns to 294ns per op. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace the math/big-based Mul (~160ns, 5 allocs) with a pure int64 schoolbook multiplication in base 10^9 (~10ns, 0 allocs), achieving ~16x speedup. The old implementation is preserved as MulSlow for reference and testing. Includes TestMulVsMulSlow with 17 table-driven cases and 10k random iterations confirming exact equivalence. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Early-return ZERO when either operand is zero, instead of letting a wrong `negative` flag propagate through the function. Fix stale comment that referenced incorrect digit indices. Expand test suite with zero*negative cases, large hi values (a[0]>0 in base-10^9), overflow cases, and a second random loop covering hi up to maxHi. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace string-based float64 conversion with strconv.AppendFloat into a [64]byte stack buffer and manual byte parsing, eliminating all heap allocations. Rename the old implementation to NewFSlow for comparison. ~2.2x faster (50ns vs 111ns), 0 allocs (down from 1 alloc/24 bytes). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…gorithm D) Replaces math/big-based division with stack-only int64 arithmetic, achieving ~4x speedup (64 ns/op vs 258 ns/op) and zero allocations. Old implementation preserved as DivSlow for reference and testing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace fmt.Sprintf + strconv.FormatInt + string concatenation with right-to-left digit writing into a [40]byte stack buffer. Reduces String/StringN/MarshalJSON from 4 allocs (80 B) to 1 alloc (32 B) and improves throughput ~3x. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
160 test cases covering String(), StringN(), MarshalJSON() across edge cases: zero-padded fractions, min/max values, round-trips, arithmetic results, point position, output length bounds, and JSON marshal/unmarshal consistency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Redesigns the fixed-point library from single int64 (10.8 digits) to dual int64 (18.18 digits) to support extended range and precision.
Changes
fp int64tohi int64, lo int64(16 bytes total)Performance
Test Results
✅ All 29 tests passing
✅ Extended precision validated:
123456789012345678.123456789012345678✅ Division accuracy:
2/3 = 0.666666666666666667(18 digits)✅ Serialization round-trip working correctly
Breaking Changes
🤖 Generated with Claude Code