-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Fix initrand to avoid random number sequences overlapping #18744
Changes from all commits
300614b
fbdff68
7898483
15eb7b4
3c14094
a3278e4
857623d
97ee811
8b218ab
a254cca
42c28cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,9 +23,10 @@ proc main() = | |
doAssert a in [[0,1], [1,0]] | ||
|
||
doAssert rand(0) == 0 | ||
doAssert sample("a") == 'a' | ||
when not defined(nimscript): | ||
doAssert sample("a") == 'a' | ||
|
||
when compileOption("rangeChecks"): | ||
when compileOption("rangeChecks") and not defined(nimscript): | ||
doAssertRaises(RangeDefect): | ||
discard rand(-1) | ||
|
||
|
@@ -92,7 +93,7 @@ block: # random int | |
|
||
block: # again gives new numbers | ||
var rand1 = rand(1000000) | ||
when not defined(js): | ||
when not (defined(js) or defined(nimscript)): | ||
os.sleep(200) | ||
|
||
var rand2 = rand(1000000) | ||
|
@@ -122,7 +123,7 @@ block: # random float | |
|
||
block: # again gives new numbers | ||
var rand1: float = rand(1000000.0) | ||
when not defined(js): | ||
when not (defined(js) or defined(nimscript)): | ||
os.sleep(200) | ||
|
||
var rand2: float = rand(1000000.0) | ||
|
@@ -248,3 +249,26 @@ block: # bug #17670 | |
type UInt48 = range[0'u64..2'u64^48-1] | ||
let x = rand(UInt48) | ||
doAssert x is UInt48 | ||
|
||
block: # bug #17898 | ||
# Checks whether `initRand()` generates unique states. | ||
# size should be 2^64, but we don't have time and space. | ||
|
||
# Disable this test for js until js gets proper skipRandomNumbers. | ||
when not defined(js): | ||
const size = 1000 | ||
var | ||
rands: array[size, Rand] | ||
randSet: HashSet[Rand] | ||
for i in 0..<size: | ||
rands[i] = initRand() | ||
randSet.incl rands[i] | ||
|
||
doAssert randSet.len == size | ||
|
||
# Checks random number sequences overlapping. | ||
const numRepeat = 100 | ||
for i in 0..<size: | ||
for j in 0..<numRepeat: | ||
discard rands[i].next | ||
doAssert rands[i] notin randSet | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be addressed in a follow up PR but the notion that a random sequence of numbers doesn't "overlap" with some other sequence seems to weaken the "randomness" and not to improve it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i don't think think this is correct; the sequences of numbers generated can have overlap in parts, eg: but the underlying states won't overlap (so long we remain within a period); if they did, the sequences generated would be identical at any point after which the state overlaps There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you tell me why is it weaken randomness? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this nested proc to top-level scope, simpler/cleaner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why moving nested proc to top-level scope makes simpler/cleaner?
It doesn't reduce a total number of lines of random module.
getRandomState
proc is used only ininitRand()
.