-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add functions for randomly generating bigints #118
Conversation
@narimiran Do you need @konsumlamm or @pietroppeter approval for PR to be merged ? |
I would assume he does not need my approval but anyway I gave a look and it looks a nice improvement to me. :) the trick on having the bound on last two limbs seems very smart but it is not evidently correct to me. will we be able to reach all possible random numbers in the interval (and in a uniform way)? I guess the answer is positive. maybe it is possible to add some kind of test or other type of assurances about this? as a follow up later (not in any way a blocker for this) we might add a |
Maybe my mental picture will make it more evidently correct. Think of the random number like a slot machine where the digits are each randomly and independently generated. First you pull the lever and all the numbers start spinning. Then the top digit falls into place. If it is too high, you pull the lever again (and due to the assumption of independence on the RNG, you don’t need to wait for all the other digits to settle). If it is equal to the high digit of the spread, you need to wait for more digits fall into place to see if the whole number is in range (this is exceedingly rare anyway - the odds you need to look at more than the first two digits depend on the spread but are always less than 1:UINT32MAX). The weak part of this PR is definitely the tests. I chose the numbers in there pretty arbitrarily, and don’t check any number-theoretic properties - just rough uniformity. |
this indeed helped me understand it better, indeed I can see as essentially working with two "digits".
|
@pietroppeter That's exactly what I have done in #112. We decided this to not be part of the public API though, but as part of utilities/testing functions for the library development.
Bigints can be seen as a sequence of digits in the base A priori we only have to generate randomly all var limbs = newSeqWith(nFullLimbs, r.rand(uint32.low..uint32.high)) @rotu This does not seem to correspond to your slot machine story. I am pretty sure you generate randomly all the lowest digits first and not the top ones, i.e. this should generate I realized after the PR that we could actually leverage my random implementation to generate in a specific slice, since what I have done generate a BigInt of a specific limb size, but could be used to generate a random number in Since this is what you end up doing for the slice, I could have tweaked my implementation. But I really like yours which is simpler and a more direct approach to the Slice random generation. Notice it is not that hard to do a doWhile construct with a template, but I am not sure it is a good idea to add to the library. template doWhile(cond, loop: untyped): untyped =
loop
while cond:
loop |
Exactly. I wanted to guarantee that the odds of having to reroll everything were low (at worst 1:2^32). If the spread were a power of 2^32, then only generating the top limb "to order" would result in rerolling everything half the time (when the top limb is generated as 1, and the second limb is anything other than 0). I originally considered generating the most significant 32 bits separately regardless of alignment. This led to less elegant code. I like that this approach does not involve any bit masking nor any knowledge of the internal structure of
You're right, but that's hella confusing! The do {
stuff();
} while (condition) Yours looks like: doWhile(condition):
stuff() If I saw the latter, I would still assume the condition got executed before, not after, the loop body! |
No description provided.