On Mon, 28 Dec 2015 11:08:56 -0700, Phil Steitz wrote:
The significant refactoring to eliminate the (standard) next(int)
included in these changes has the possibility of introducing subtle
bugs or performance issues.  Please run some tests to verify that
the same sequences are generated by the 3_X code

IIUC our unit tests of the RNGs, this is covered.

and the refactored
code and benchmarks to show there is no loss in performance.

Given that there are exactly two operations _less_ (a subtraction
and a shift), it would be surprising.

It
would also be good to have some additional review of this code by
PRNG experts.

The "nextInt()" code is exactly the same as the "next(int)" modulo
the little change above (in the last line of the "nextInt/next" code).

That change in "nextInt/next" implied similarly tiny recodings in
the generic methods "nextDouble()", "nextBoolean()", ... which, apart
from that, were copied from "BitsStreamGenerator".

[However tiny a change, I had made a mistake... and dozens of tests
started to fail. Found the typo and all was quiet again...]

About "next(int)" being standard, it would be interesting to know
what that means.
As I indicated quite clearly in one of my first posts about this
refactoring
1. all the CM implementations generate random bits in batches
   of 32 bits, and
2. before returning, the "next(int bits)" method was truncating
   the generated "int":
     return x >>> (32 - bits);

In all implementations, that was the only place where the "bits"
parameter was used, from which I concluded that the randomness
provider does not care if the request was to create less than 32
random bits.
Taking "nextBoolean()" for example, it looks like a waste of 31
bits (or am I missing something?).

Of course, if some implementation were able to store the bits not
requested by the last call to "next(int)", then I'd understand that
we must really provide access to a "next(int)" method.

Perhaps that the overhead of such bookkeeping is why the practical
algorithms chose to store integers rather than bits (?).

As you dismissed my request about CM being able to care for a RNG
implementation based on a "long", I don't quite understand the
caring for a "next(int)" that serves no more purpose (as of current
CM).


Gilles


Phil

On 12/28/15 10:23 AM, er...@apache.org wrote:
Repository: commons-math
Updated Branches:
  refs/heads/master 7b62d0155 -> 81585a3c4


MATH-1307

New base class for RNG implementations.
The source of randomness is provided through the "nextInt()" method (to be defined in subclasses).


[...]


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to