Closed Bug 1239479 Opened 4 years ago Closed 4 years ago

Add documentation in response to bug 1206356 comment 10's details about XorShift128PlusRNG


(Core :: MFBT, defect, minor)

Not set





(Reporter: Waldo, Assigned: jimb)


(Blocks 1 open bug)



(1 file)

Bug 1206356 comment 10 describes some mathematical properties of the algorithm, that we'd do well to put in comments.  It also suggests an initialization algorithm that'd let us get rid of the manual don't-init-to-zeroes stuff we have.  Probably worth doing both -- separate patches/landings.
Here are Waldo's comments, revised per IRC discussion.
Attachment #8707642 - Flags: review?(jwalden+bmo)
Comment on attachment 8707642 [details] [diff] [review]
Add comments to mfbt/XorShift128PlusRNG.h from the RNG's designer.

Review of attachment 8707642 [details] [diff] [review]:

::: mfbt/XorShift128PlusRNG.h
@@ +79,5 @@
>    /*
> +   * Return a pseudo-random floating-point value in the range [0, 1). More
> +   * precisely, choose an integer in the range [0, 2**53) and divide it by
> +   * 2**53. Given the 2**128 - 1 period noted above, the produced doubles are
> +   * all but uniformly distributed in this range.

I didn't much like the "all but uniformly distributed" bit here when I wrote it, but I wanted to be precise and not claim a truly uniform distribution (given the 2**64 - 1 thing necessarily makes it not wholly uniform).  If you have better ideas here, go for 'em.
Attachment #8707642 - Flags: review?(jwalden+bmo) → review+
Flags: in-testsuite-
OS: Unspecified → All
Hardware: Unspecified → All
Target Milestone: --- → mozilla46
Keywords: leave-open
I'm not taking on the task of improving the seeding process, so I'm de-assigning myself from this bug.
Assignee: jimb → nobody
Keywords: leave-open
Summary: Respond to bug 1206356 comment 10's details about XorShift128PlusRNG → Add documentation in response to bug 1206356 comment 10's details about XorShift128PlusRNG
Assignee: nobody → jimb
Attachment #8707642 - Flags: checkin+
I've split the seeding work off into bug 1244784, so this can be closed.
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.