You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
MillerShuffle(3, 612489217, 10) is -5 , which I would not expect. I thought the output should be between 0 and 9, inclusive.
I believe this happens because the math was copied over from C, but the C code is using unsigned int and JavaScript ^ on "number" uses essentially signed 32 bit, if I'm understanding correctly.
So the calculation for r2 for those input values ends up negative, which doesn't look intended.
r2=((randR*0x89)^r1)%p2;
The naive fix might be to do that ^ in BigInt and then truncate with BigInt.asUintN(32,...) and get back to number-land that way? Eg:
I'm not 100% sure if that breaks the logic or if there's other places that can similarly overflow. Safest might be doing all of the math in BigInt, truncating with asUintN(32,...) at many of the steps and then converting to number at the end, but I bet many of them are unnecessary, it's just not obvious to me which need it and which don't.
The only one that obviously does need it to me is that part of the calculation of r2 that I showed above.
The text was updated successfully, but these errors were encountered:
kadoban
added a commit
to kadoban/Miller_Shuffle_Algo
that referenced
this issue
Jan 2, 2025
MillerShuffle(3, 612489217, 10)
is-5
, which I would not expect. I thought the output should be between 0 and 9, inclusive.I believe this happens because the math was copied over from C, but the C code is using unsigned int and JavaScript
^
on "number" uses essentially signed 32 bit, if I'm understanding correctly.So the calculation for r2 for those input values ends up negative, which doesn't look intended.
r2=((randR*0x89)^r1)%p2;
The naive fix might be to do that
^
in BigInt and then truncate withBigInt.asUintN(32,...)
and get back to number-land that way? Eg:r2=Number(BigInt.asUintN(32, BigInt(randR*0x89)^BigInt(r1)))%p2;
I'm not 100% sure if that breaks the logic or if there's other places that can similarly overflow. Safest might be doing all of the math in BigInt, truncating with
asUintN(32,...)
at many of the steps and then converting to number at the end, but I bet many of them are unnecessary, it's just not obvious to me which need it and which don't.The only one that obviously does need it to me is that part of the calculation of r2 that I showed above.
The text was updated successfully, but these errors were encountered: