Skip to content

Re-round coefficient when a rounding carry exceeds precision (#236)#238

Open
b-erdem wants to merge 1 commit into
ericmj:mainfrom
b-erdem:fix-rounding-carry-precision
Open

Re-round coefficient when a rounding carry exceeds precision (#236)#238
b-erdem wants to merge 1 commit into
ericmj:mainfrom
b-erdem:fix-rounding-carry-precision

Conversation

@b-erdem

@b-erdem b-erdem commented Jul 5, 2026

Copy link
Copy Markdown

Fixes the rounding-carry half of #236.

Problem

When rounding carries an all-nines coefficient up, the coefficient gains a digit and the result is left with precision + 1 significant digits instead of being re-rounded to precision:

Context.with(%Context{precision: 1}, fn -> Decimal.div(~d"95", ~d"10") end)
#=> Decimal.new("10")     # coef 10 at precision 1; should be 1E+1

Per the General Decimal Arithmetic spec's round operation, a carry that lengthens the coefficient past precision must drop the trailing zero and raise the exponent. Python's decimal agrees (Decimal(95)/Decimal(10) at precision 1 → 1E+1).

Fix

do_precision/7 drops the trailing zero the carry introduced and bumps the exponent, keeping exactly precision significant digits.

Scope — please note

This is deliberately in the shared context-rounding path, so it corrects the carry for add, sub, mult, and div, not just division. That means a few existing context_test.exs cases were pinning the old precision + 1-digit results and are updated here:

  • with_context/2: half evenadd(0, 9.99) at precision 2 → d(1, 10, 0) (was d(1, 100, -1)).
  • with_context/2: large exponent gap subtraction and … stays bounded — these are the large-gap sticky-bit paths from the CVE‑2026‑32686 mitigation. The carry modes now yield d(1, 100, 99_998) (three significant digits) rather than d(1, 1000, 99_997). :down/:floor are unchanged (they truncate, no carry). I flag these specifically because they touch the security-mitigation code — the values are numerically identical, only the representation is corrected to stay within precision.

Each updated expectation was checked against Python's decimal.

Verification

Three-way differential (this branch vs. unpatched main vs. Python) over ~10k carry-triggering cases across the four operations and seven rounding modes:

  • 10,180 cases corrected to match Python (previously kept the extra digit).
  • 0 cases moved away from Python. The only residual differences from Python are a pre-existing :up rounding quirk (exact 9.0 rounding to 10 at precision 1) that this PR does not touch — in every such case the value was already produced by main and is merely re-represented; it is not introduced here. Happy to file that separately if useful.

Tests

  • New context_test.exs test: carry re-round across div/add/mult/sub and :ceiling, all matching the spec/Python.
  • Updated the three cases above with explanatory comments.

All tests pass (101 doctests, 27 properties, 102 tests).

When rounding an all-nines coefficient up (9.99 at precision 2, or
Decimal.div(95, 10) at precision 1), digits_increment lengthened the
coefficient by one digit (9.99 -> 10.0, 9.5 -> 10), leaving a result
with precision + 1 significant digits: d(1, 100, -1) and d(1, 10, 0)
instead of d(1, 10, 0) and d(1, 1, 1).

do_precision now drops the trailing zero the carry introduced and
raises the exponent, keeping exactly `precision` significant digits per
the General Decimal Arithmetic spec's round operation. This is the
shared context-rounding path, so it applies to add, sub, mult, and div;
the updated context_test cases (including the large-exponent-gap
subtraction from the CVE-2026-32686 mitigation) reflect the corrected
three-significant-digit results.

Verified against Python's decimal: on ~10k carry-triggering cases across
the four operations and seven rounding modes, every result my change
touches now matches the reference (previously all kept the extra digit).
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