Skip to content

Commit

Permalink
Use division instead of modulus in RangeInt
Browse files Browse the repository at this point in the history
  • Loading branch information
pitdicker committed Dec 7, 2017
1 parent b8fb05c commit cb0f3f8
Showing 1 changed file with 10 additions and 5 deletions.
15 changes: 10 additions & 5 deletions src/distributions/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,13 @@ macro_rules! range_int_impl {
// integers that can be generated to this range. We have to map
// integers from a `zone` that is a multiple of the range. The
// rest of the integers, that cause a bias, are rejected. The
// sampled number is `zone % range`.
// sampled number is `zone / range`.
//
// The problem with `range` is that to cover the full range of
// the type, it has to store `unsigned_max + 1`, which can't be
// represented. But if the range covers the full range of the
// type, no modulus is needed. A range of size 0 can't exist, so
// we use that to represent this special case. Wrapping
// type, no division is needed. A range of size 0 can't exist,
// so we use that to represent this special case. Wrapping
// arithmetic even makes representing `unsigned_max + 1` as 0
// simple.
//
Expand All @@ -159,7 +159,6 @@ macro_rules! range_int_impl {
// small integer. For an u8 it only ever uses 7 bits. This means
// that all but the last 7 bits of `zone` are always 1's (or 15
// in the case of u16). So nothing is lost by trucating `zone`.

let unsigned_max: $u_large = ::core::$u_large::MAX;

let range = (high as $u_large)
Expand Down Expand Up @@ -193,7 +192,13 @@ macro_rules! range_int_impl {
loop {
let v: $u_large = Rand::rand(rng, Uniform);
if v <= zone {
return self.low.wrapping_add((v % range) as $ty);
// Using a modulus could work just as well as a
// division. With division the result depends more
// on the most significant bits, with modulus on the
// least significant bits. Because many RNG's of
// lower quality are weak in their least significant
// bits, we use division.
return self.low.wrapping_add((v / range) as $ty);
}
}
} else {
Expand Down

0 comments on commit cb0f3f8

Please sign in to comment.