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
Increase the period for rand, fix negative time #905
Comments
@yaxu I don't fully remember why I settled on 300 ages ago... but doing a few quick tests now, if you stretch the algorithm over 300000 or 3000000 cycles, and then sample at 1/16 of a cycle, there is a clear pattern of cyclical behaviour. Here's at 300000:
30000 looks "better" - although if one were to sample the algorithm ten times faster I guess the same issue would appear:
So I guess 300 was probably something like a number that was sure to push this potential for short-term cyclic behaviour far out of salience? Another thought might be to make this user configurable - a parameter that is read from the environment. If another parameter, some kind of offset,was also added so that there were two parameters controlling the generator that might retain the blinding speed of this algorithm, while also adding a somewhat more intuitive UI for having multiple random streams, while also addressing this issue (when it is an issue). Re: the negative time issue... timeToRand seems to work fine with negative times, so I suspect it is something to do with the way it is currently integrated into rand:
I am a bit suspicious of that double conversion in the middle of things there. Could that be doing weird things, and might changing it to Rational help? |
I did a bit of digging last weekend
while what we do is something like
Unfortunately the My suggestion is to use a hash function to randomise the time seed The implementation that is currently in pull request #942 |
This looks great to me. Thanks @fbous !!! |
I've noticed that
rand
has a period of 300 cycles.I haven't heard of anyone noticing it loop, but it's feasible that someone could under some conditions.
@dktr0 do you see an advantage in setting at that, rather than something a few orders of magnitude higher?
Also what looks more like a bug, negative time gives negative numbers:
The text was updated successfully, but these errors were encountered: