Re-round coefficient when a rounding carry exceeds precision (#236)#238
Open
b-erdem wants to merge 1 commit into
Open
Re-round coefficient when a rounding carry exceeds precision (#236)#238b-erdem wants to merge 1 commit into
b-erdem wants to merge 1 commit into
Conversation
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).
This was referenced Jul 5, 2026
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.
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 + 1significant digits instead of being re-rounded toprecision:Per the General Decimal Arithmetic spec's round operation, a carry that lengthens the coefficient past
precisionmust drop the trailing zero and raise the exponent. Python'sdecimalagrees (Decimal(95)/Decimal(10)at precision 1 →1E+1).Fix
do_precision/7drops the trailing zero the carry introduced and bumps the exponent, keeping exactlyprecisionsignificant digits.Scope — please note
This is deliberately in the shared context-rounding path, so it corrects the carry for
add,sub,mult, anddiv, not just division. That means a few existingcontext_test.exscases were pinning the oldprecision + 1-digit results and are updated here:with_context/2: half even—add(0, 9.99)at precision 2 →d(1, 10, 0)(wasd(1, 100, -1)).with_context/2: large exponent gap subtractionand… stays bounded— these are the large-gap sticky-bit paths from the CVE‑2026‑32686 mitigation. The carry modes now yieldd(1, 100, 99_998)(three significant digits) rather thand(1, 1000, 99_997).:down/:floorare 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 withinprecision.Each updated expectation was checked against Python's
decimal.Verification
Three-way differential (this branch vs. unpatched
mainvs. Python) over ~10k carry-triggering cases across the four operations and seven rounding modes::uprounding quirk (exact9.0rounding to10at precision 1) that this PR does not touch — in every such case the value was already produced bymainand is merely re-represented; it is not introduced here. Happy to file that separately if useful.Tests
context_test.exstest: carry re-round acrossdiv/add/mult/suband:ceiling, all matching the spec/Python.All tests pass (
101 doctests, 27 properties, 102 tests).