peterolson / BigInteger.js

An arbitrary length integer library for Javascript
The Unlicense
1.12k stars 187 forks source link

bigInt.randBetween issue #221

Closed thearthouse closed 2 years ago

thearthouse commented 2 years ago

why it's never hit a number 33185509 when using bigInt.randBetween("16777216", "33554431");

Yaffle commented 2 years ago

Looks like it will never be bigger 26777215 (which is 10**7 + 16777216 - 1), while it should produce such numbers almost half of times. (it is only reproducisble when platform supports native bigints)

thearthouse commented 2 years ago

looks like it will never be bigger 26777215 (which is 10**7 + 16777216 - 1) only when BigInt is supported

also tested bigInt.randBetween(BigInt("16777216"), BigInt("33554431")); result is same

Yaffle commented 2 years ago

seems, https://github.com/peterolson/BigInteger.js/blob/master/BigInteger.js#L1181 this line should have digits[i] + 1:

-            var top = restricted ? digits[i] : BASE;
+           var top = restricted ? digits[i] + 1 : BASE;
thearthouse commented 2 years ago

seems, https://github.com/peterolson/BigInteger.js/blob/master/BigInteger.js#L1181 this line should have digits[i] + 1:

-            var top = restricted ? digits[i] : BASE;
+           var top = restricted ? digits[i] + 1 : BASE;

i change that and its working fine , will be there any problem on high integers like 2**120

peterolson commented 2 years ago

@Yaffle Can you make a pull request? I'll merge it in

Yaffle commented 2 years ago

ok

Yaffle commented 2 years ago

@peterolson a test is falling and I don't know how to resolve it :-)

peterolson commented 2 years ago

Which test?

Yaffle commented 2 years ago

@peterolson , i have added more ranges to "is within 10% of uniform distribution (this test is probabilistic and has a small change of failing)"

Yaffle commented 2 years ago

@peterolson see https://github.com/peterolson/BigInteger.js/pull/222/files

thearthouse commented 2 years ago

@peterolson can you answer will any problem on high numbers?

Yaffle commented 2 years ago

@peterolson I have created a new pull request, please look - https://github.com/peterolson/BigInteger.js/pull/223/files

peterolson commented 2 years ago

Fixed with #223.