On Sun, 27 Dec 2015 11:15:42 -0700, Phil Steitz wrote:
On 12/27/15 6:51 AM, Gilles wrote:
On Sun, 27 Dec 2015 12:48:08 +0100, Gilles wrote:
Hi.
[...]
BitStreamGenerator uses the more standard int next(int bits).
Note
that we have *no* internal direct implementations of
AbstractRandomGenerator, while BitStreamGenerator has worked very
nicely as a root for good PRNGs.
Something implicit in "BitStreamGenerator": the maximum number of
bits is 32 (cf. return type of "next(int)" and the ubiquitous use
of hard-coded "32".
What about the possibility of using a "long" as a building block
for another family of RNG?
Gilles
Therefore, I think for V4 it might actually be best to just drop
AbstractRandomGenerator. Sorry I did not think of this before
responding above.
Phil
(1)
Do all RNG that are going to ever be implemented in CM based on a
"int next(int)" method?
If not, then we can have a problem as soon as there is another
way to provide the random bits.
(2)
Assuming that we only need
int next(int bits)
I notice that the source of random bits does indeed creates an
"int",
say "x", and then truncates it before returning:
protected int next(int bits) {
// Build "x"...
return x >>> 32 - bits;
}
So, I believe that the actual basis for generating randomness
should be
a "public abstract int nextInt()" method to be overridden by
concrete
RNG producers (as "next(int)" currently is):
@Override
public int nextInt() {
// Build "x"...
return x;
}
and the truncation should be performed inside the respective
methods that
currently call "next(int)". [Which are all in
"BitsStreamGenerator".]
For example: replace the current
public float nextFloat() {
return next(23) * 0x1.0p-23f;
}
by
public float nextFloat() {
return (nextInt() >>> 9) * 0x1.0p-23f;
}
And, as a bonus, this will avoid the unnecessary
x >>> (32 - 32)
operation currently performed when calling "next(32)".
OK?
If so, I propose to still create a new "BaseRandomGenerator"
class that will
be different from "BitsStreamGenerator" because of the above
changes.
This class should be backported to MATH_3_X so that we can
deprecate
"BitsStreamGenerator" in 3.6.
In addition to points (1) and (2) above, we should discuss the
following one.
(3)
Deprecate the "setSeed" methods (and also remove them from the
"RandomGenerator" interface).
-1
The RandomGenerator interface is extracted from Random. As such, it
provides a simple way to plug in a [math]-supplied PRNG where Random
is used in code. Dropping this method will force a lot of needless
refactoring. The setSeed method exists as a convenience for users.
As one of those users, I don't want to see it go away.
Never mind; I can fix the constructor safety problem with new
private void setSeedInternal(/* ... */)
methods that will be called by their public counterparts.
Do we attempt to make the RNGs thread-safe?
It would mean that the "setSeed" methods must be made "synchronized".
More importantly, I'd like opinions on points (1) and (2) above...
In fact, you gave a partial answer to (1) in the other post, indicating
that CM does not have implementations other than the one producing an
"int", which I knew: The purpose of asking was whether we should be
expecting some change (like OS that went from 32 bits to 64 bits...).
If we care only for the present, then proposal (2) is OK I think.
Please confirm.
Gilles
Phil
The rationale is that they typically provide information for a
constructor;
they invalidate the previous state of the instance and imply the
same
computation as is performed at construction.
[The only task not repeated by "setSeed" is the array(s)
allocation.]
I thus propose that we use the fluent API, as we agreed to do in
general,
for the RNGs too. [I.e. a method "withSeed" will create a new
instance.]
This will also solve the potential security problem of having
public methods
called from the constructor.
Any objection?
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
