Skip to content

Correctness fixes - #242

Open
tomciopp wants to merge 3 commits into
ericmj:mainfrom
tomciopp:correctness-fixes
Open

Correctness fixes#242
tomciopp wants to merge 3 commits into
ericmj:mainfrom
tomciopp:correctness-fixes

Conversation

@tomciopp

Copy link
Copy Markdown
Contributor

Three correctness fixes, found by auditing the rounding paths after the
recent performance work. Each is its own commit with a pinning test. All
are wrong-answer bugs at the default context, reachable through the
public API with in-range decimal128 operands.

to_float/1: off by one for odd integers in [2^52, 2^53)

decimal_to_float/4 rounded by comparing the remainder against
den >>> 1. When scale_down collapses the denominator to 1, every
input with a 53-bit significand, the division is exact, but the zero
remainder fails both rem > den and rem < den against a halved
denominator of 0 and falls through to the ties-to-even clause, which
increments every odd quotient:

Decimal.to_float(Decimal.new(4_503_599_627_370_497))
#=> 4503599627370498.0 (the exact value is representable)

Fixed by comparing the doubled remainder against the denominator, which
is the rounding step as given in the algorithm the implementation
follows ("Correct Decimal to Floating-Point Using Big Integers",
exploringbinary.com, cited in to_float/1), restoring IEEE 754
correctly-rounded conversion. Odd values just above 2^53, where a zero
scaled remainder is a genuine tie, still round to even.

sqrt/1: discarded digits were not carried into rounding

The inexact branch of do_sqrt/5 passed its truncated root to the
context with the sticky bit clear, so rounding treated the guard digit
as the entire discarded part, the same defect class as the division
sticky-bit fix (#236). The true root lies strictly beyond the truncated
coefficient on that branch, so:

  • directed modes stopped short when the guard digit was 0: at
    precision 9 under :ceiling, sqrt(10) returned 3.16227766, though
    √10 = 3.16227766016… ceils to 3.16227767
  • a guard digit of 5 with digits beyond it became a false half-even
    tie: at precision 2 under :half_even, sqrt(1.57) returned 1.2
    instead of 1.3 (√1.57 = 1.25299…)
  • inexact roots signalled only :rounded; they now also signal
    :inexact

The default :half_up was unaffected (the guard digit alone decides
there), which is how this went unnoticed.

rem/2 and div_rem/2: remainder computed against a rounded product

Both derived the remainder as sub(num1, mult(num2, quotient)), and
mult/2 applies the context, so when divisor × quotient exceeded the
context precision, the remainder was computed against a rounded
multiple of the divisor. With 34-digit operands the rounded product can
equal the dividend exactly, cancelling the remainder:

Decimal.rem("9999999999999999999999999999999999",
            "2.000000000000000000000000000000001")
#=> 0, true remainder 3E-33

The remainder is now computed exactly on raw coefficients (multiply,
align, subtract), with only the final result passing through the
context, per the General Decimal Arithmetic spec's requirement that
remainder is exact. Intermediates stay input-proportional because
integer_division/5 caps the quotient at precision + 1 digits.

Behavior note: a zero remainder now takes the dividend's sign
(rem(-4, 2) is -0, previously +0), matching IEEE 754 and Python's
decimal — it falls out of the exact computation, since the quotient is
floor(|num1| / |num2|).

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant