fix: round zeta BigNumber digit count to avoid non-integer factorial - #3683
xianjianlf2 wants to merge 2 commits into
Conversation
The BigNumber branch of zeta derived its digit count from Math.abs(Math.log10(config.relTol)). When relTol is not an exact power of ten (e.g. 5e-8, or 1e-320 where floating point rounding makes log10 non-integer), this produced a non-integer digit count that eventually reached factorial/gamma as a non-integer BigNumber, throwing "Integer BigNumber expected". Round the value so the digit count is always an integer. Closes josdejong#3532
af74322 to
df7e666
Compare
…er-digits-round-3532
lbesecker195
left a comment
There was a problem hiding this comment.
Ran zeta in BigNumber mode from src/defaultInstance.js on develop and on this branch. On develop, relTol: 5e-16, relTol: 2e-10 and the issue's precision: 400, relTol: 1e-320 all throw Integer BigNumber expected, because Math.abs(Math.log10(relTol)) is 15.30…, 9.69… and 320.0000000000001 there. With Math.round all three compute, and the 1e-320 case agrees with zeta(3) to the ~38 digits I printed. A power-of-ten tolerance like 1e-12 gives exactly the same result as before, since rounding doesn't change an exact integer. mocha test/unit-tests/function/special/zeta.test.js goes from 6 to 7 passing and eslint is clean.
Side note, not something this PR introduces: the BigNumber results are only good to about 1e-10 here (zeta(3) with relTol: 1e-12 is already off at the 9th digit on develop), which I take to be the separate inaccuracy tracked in #3551. LGTM for the crash.
Problem
The BigNumber branch of
zetaderived its digit count fromMath.abs(Math.log10(config.relTol)). WhenrelTolis not an exact power of ten (e.g.5e-8, or1e-320where floating-point rounding makeslog10non-integer), this produced a non-integer digit count. That value eventually reachedfactorial/gammaas a non-integer BigNumber, throwingInteger BigNumber expected.Fix
Round the derived value with
Math.round(...)so the digit count is always an integer, regardless of floating-point noise inMath.log10.Testing
Added a unit test in
test/unit-tests/function/special/zeta.test.jsthat configures BigNumber mode with arelTolwhose base-10 logarithm is non-integer (5e-16) and asserts thatzeta(3)no longer throws and returns the expected value.Closes #3532