Skip to content

Fix toString(base) under-rounding for odd bases (#412) - #413

Open
youdie006 wants to merge 1 commit into
MikeMcl:mainfrom
youdie006:fix/412-tostring-odd-base-rounding
Open

Fix toString(base) under-rounding for odd bases (#412)#413
youdie006 wants to merge 1 commit into
MikeMcl:mainfrom
youdie006:fix/412-tostring-odd-base-rounding

Conversation

@youdie006

Copy link
Copy Markdown

Fixes #412.

Problem

toString(base) under-rounds for odd bases. Under { DECIMAL_PLACES: 0, ROUNDING_MODE: ROUND_HALF_UP }:

new BigNumber('0.6').toString(3)   // '0'  (should be '1')
new BigNumber('2.6').toString(3)   // '2'  (should be '10')

.toString(10) is correct, so base 10 is the oracle. The values are exact (string constructor), so this is not float error.

Root cause

In convertBase the round-up decision compared the single rounding digit i against k = baseOut / 2. For an odd baseOut, k is fractional (1.5 for base 3), so i == k is unreachable and the remainder flag is never consulted: a discarded tail whose leading digit is floor(baseOut/2) was treated as "< half" even when the full tail was >= half a ULP.

A local tweak isn't sufficient: for an odd base, half a ULP is 0.mmm... (m = (baseOut-1)/2), a non-terminating fraction, so the tail must be compared against a repeating m -- but div only produced ~2 guard digits, giving byte-identical output for values that must round differently (e.g. base-3 0.49->0 vs 0.51->1 both yield [1,1,1]).

Fix

  1. For an odd output base, compute enough extra base-baseOut digits that the first tail digit differing from m is always available. The leading run of m digits before the tail is decided is at most fracLen * log_baseOut(10).
  2. Replace the round-up decision with a base-parity-aware one:
    • directed modes (UP/DOWN/CEIL/FLOOR) are parity-independent -- unchanged;
    • even bases keep the exact original logic (byte-identical output);
    • odd bases compare the tail digit-by-digit against m to get its position relative to half. For ROUND_HALF_EVEN on an exact half, the even neighbour has an even mantissa, which (baseOut being odd) means an even sum of kept digits -- generalising the even-base xc[d-1] & 1 test.

Even-base and decimal output are unchanged.

Tests

  • The two reported cases plus an inline exact BigInt oracle cross-check over odd bases {3,5,7,9} (and even controls {2,4,8,16}) for every rounding mode {0..8}.
  • Full suite: 71133 of 71133 pass (node 22).
  • Cross-checked against an independent exact-rational oracle over ~160k cases (random, half-boundary adversarial, large fraction lengths) -- 0 mismatches, and 0 changes to even-base output.
  • Four auto-generated odd-base fixtures were each off by one ULP (they had recorded the old buggy output); corrected to the exact values.

dist/ was rebuilt with node build.js.


This fix was prepared with AI assistance (Claude) and reviewed by me before submission.

toString(base) under-rounded for odd bases: under DECIMAL_PLACES 0 / ROUND_HALF_UP,
new BigNumber('0.6').toString(3) gave '0' (should be '1') and '2.6'.toString(3) gave
'2' (should be '10'), while .toString(10) is correct.

In convertBase the round-up decision compared the single rounding digit against
k = baseOut/2, which is fractional for odd bases, so the i == k branch was
unreachable and the remainder flag was never consulted. A local tweak is
insufficient: for an odd base half a ULP is 0.mmm... (m = (baseOut-1)/2), so the
tail must be compared against a repeating m, but div only produced ~2 guard digits.

Fix: (1) for an odd output base from toString, compute enough extra base-baseOut
digits (bounded by fracLen*log_baseOut(10)) that the first tail digit differing from
m is available; (2) replace the round-up decision with a base-parity-aware one -
directed modes unchanged, even bases byte-identical, odd bases compare the tail
digit-by-digit against m, with HALF_EVEN using even-mantissa (even sum of kept
digits, since baseOut is odd). Even-base and decimal output are unchanged.

Fixes MikeMcl#412
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.

toString(base) under-rounds for odd bases

1 participant