You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by er...@apache.org on 2015/12/28 18:23:13 UTC
[01/18] [math] MATH-1307
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).
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/4cbb388b
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/4cbb388b
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/4cbb388b
Branch: refs/heads/master
Commit: 4cbb388ba9099be121f81d75000acc3af93bf993
Parents: 7b62d01
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 16:42:55 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 16:42:55 2015 +0100
----------------------------------------------------------------------
.../math4/random/BaseRandomGenerator.java | 270 +++++++++++++++++++
.../math4/random/BaseRandomGeneratorTest.java | 129 +++++++++
2 files changed, 399 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/4cbb388b/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java b/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
new file mode 100644
index 0000000..a78fd38
--- /dev/null
+++ b/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
@@ -0,0 +1,270 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.math4.random;
+
+import java.io.Serializable;
+
+import org.apache.commons.math4.exception.NotStrictlyPositiveException;
+import org.apache.commons.math4.exception.OutOfRangeException;
+import org.apache.commons.math4.util.FastMath;
+
+/**
+ * Abstract class implementing the methods of the {@link RandomGenerator}
+ * interface in a generic way on the basis of abstract method {@link nextInt()}
+ * to be defined in subclasses.
+ *
+ * It also provides additional utility methods that are not part of the
+ * {@link RandomGenerator} API.
+ *
+ * @since 4.0
+ */
+public abstract class BaseRandomGenerator
+ implements RandomGenerator,
+ Serializable {
+ /** Identifier for serialization. */
+ private static final long serialVersionUID = 20151227L;
+ /** Next Gaussian. */
+ private double nextGaussian = Double.NaN;
+
+ /**
+ * {@inheritDoc}
+ *
+ * Basic building block for all the generic methods defined in this class.
+ * It produces the next random number according to a specific algorithm to
+ * be implemented by a subclass.
+ */
+ @Override
+ public abstract int nextInt();
+
+ /** {@inheritDoc} */
+ @Override
+ public boolean nextBoolean() {
+ return (nextInt() >>> 31) != 0;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public double nextDouble() {
+ final long high = ((long) (nextInt() >>> 6)) << 26;
+ final int low = nextInt() >>> 6;
+ return (high | low) * 0x1.0p-52d;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public float nextFloat() {
+ return (nextInt() >>> 9) * 0x1.0p-23f;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public double nextGaussian() {
+ final double random;
+ if (Double.isNaN(nextGaussian)) {
+ // Generate a new pair of gaussian numbers.
+ final double x = nextDouble();
+ final double y = nextDouble();
+ final double alpha = 2 * FastMath.PI * x;
+ final double r = FastMath.sqrt(-2 * FastMath.log(y));
+ random = r * FastMath.cos(alpha);
+ nextGaussian = r * FastMath.sin(alpha);
+ } else {
+ // Use the second element of the pair already generated.
+ random = nextGaussian;
+ nextGaussian = Double.NaN;
+ }
+
+ return random;
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * <p>
+ * This default implementation is copied from Apache Harmony
+ * java.util.Random (r929253).
+ * </p>
+ *
+ * <p>Implementation notes:
+ * <ul>
+ * <li>If n is a power of 2, this method returns
+ * {@code (int) ((n * (long) next(31)) >> 31)}.</li>
+ * <li>If n is not a power of 2, what is returned is {@code next(31) % n}
+ * with {@code next(31)} values rejected (i.e. regenerated) until a
+ * value that is larger than the remainder of {@code Integer.MAX_VALUE / n}
+ * is generated. Rejection of this initial segment is necessary to ensure
+ * a uniform distribution.</li>
+ * </ul>
+ * </p>
+ */
+ @Override
+ public int nextInt(int n) throws IllegalArgumentException {
+ if (n > 0) {
+ if ((n & -n) == n) {
+ return (int) ((n * (long) (nextInt() >>> 1)) >> 31);
+ }
+ int bits;
+ int val;
+ do {
+ bits = (nextInt() >>> 1);
+ val = bits % n;
+ } while (bits - val + (n - 1) < 0);
+ return val;
+ }
+
+ throw new NotStrictlyPositiveException(n);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public long nextLong() {
+ final long high = ((long) nextInt()) << 32;
+ final long low = nextInt() & 0xffffffffL;
+ return high | low;
+ }
+
+ /**
+ * Returns a pseudorandom, uniformly distributed {@code long} value
+ * between 0 (inclusive) and the specified value (exclusive), drawn from
+ * this random number generator's sequence.
+ *
+ * @param n the bound on the random number to be returned. Must be
+ * positive.
+ * @return a pseudorandom, uniformly distributed {@code long} value
+ * between 0 (inclusive) and n (exclusive).
+ * @throws IllegalArgumentException if n is not positive.
+ */
+ public long nextLong(long n) {
+ if (n > 0) {
+ long bits;
+ long val;
+ do {
+ bits = ((long) (nextInt() >>> 1)) << 32;
+ bits |= ((long) nextInt()) & 0xffffffffL;
+ val = bits % n;
+ } while (bits - val + (n - 1) < 0);
+ return val;
+ }
+
+ throw new NotStrictlyPositiveException(n);
+ }
+
+ /**
+ * Clears the cache used by the default implementation of
+ * {@link #nextGaussian}.
+ */
+ public void clear() {
+ nextGaussian = Double.NaN;
+ }
+
+ /**
+ * Generates random bytes and places them into a user-supplied array.
+ *
+ * <p>
+ * The array is filled with bytes extracted from random integers generated
+ * using {@link #nextInt()}.
+ * This implies that the number of random bytes generated may be larger than
+ * the length of the byte array.
+ * </p>
+ *
+ * @param bytes Array in which to put the generated bytes. Cannot be {@code null}.
+ */
+ @Override
+ public void nextBytes(byte[] bytes) {
+ nextBytesFill(bytes, 0, bytes.length);
+ }
+
+ /**
+ * Generates random bytes and places them into a user-supplied array.
+ *
+ * <p>
+ * The array is filled with bytes extracted from random integers generated
+ * using {@link #nextInt()}.
+ * This implies that the number of random bytes generated may be larger than
+ * the length of the byte array.
+ * </p>
+ *
+ * @param bytes Array in which to put the generated bytes. Cannot be {@code null}.
+ * @param start Index at which to start inserting the generated bytes.
+ * @param len Number of bytes to insert.
+ * @throws OutOfRangeException if {@code start < 0} or {@code start >= bytes.length}.
+ * @throws OutOfRangeException if {@code len <= 0} or {@code len > bytes.length - start}.
+ */
+ public void nextBytes(byte[] bytes,
+ int start,
+ int len) {
+ if (start < 0 ||
+ start >= bytes.length) {
+ throw new OutOfRangeException(start, 0, bytes.length);
+ }
+ final int max = bytes.length - start;
+ if (len <= 0 ||
+ len > max) {
+ throw new OutOfRangeException(len, 0, max);
+ }
+
+ nextBytesFill(bytes, start, len);
+ }
+
+ /**
+ * Generates random bytes and places them into a user-supplied array.
+ *
+ * <p>
+ * The array is filled with bytes extracted from random integers generated
+ * using {@link #nextInt()}.
+ * This implies that the number of random bytes generated may be larger than
+ * the length of the byte array.
+ * </p>
+ *
+ * @param bytes Array in which to put the generated bytes. Cannot be {@code null}.
+ * @param position Index at which to start inserting the generated bytes.
+ * @param length Number of bytes to insert.
+ */
+ private void nextBytesFill(byte[] bytes,
+ int position,
+ int length) {
+ int index = position; // Index of first insertion.
+
+ // Index of first insertion plus multiple 4 part of length (i.e. length
+ // with two least significant bits unset).
+ final int indexLoopLimit = index + (length & 0x7ffffffc);
+
+ // Start filling in the byte array, 4 bytes at a time.
+ while (index < indexLoopLimit) {
+ final int random = nextInt();
+ bytes[index++] = (byte) random;
+ bytes[index++] = (byte) (random >>> 8);
+ bytes[index++] = (byte) (random >>> 16);
+ bytes[index++] = (byte) (random >>> 24);
+ }
+
+ final int indexLimit = position + length; // Index of last insertion + 1.
+
+ // Fill in the remaining bytes.
+ if (index < indexLimit) {
+ int random = nextInt();
+ while (true) {
+ bytes[index++] = (byte) random;
+ if (index < indexLimit) {
+ random >>>= 8;
+ } else {
+ break;
+ }
+ }
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/commons-math/blob/4cbb388b/src/test/java/org/apache/commons/math4/random/BaseRandomGeneratorTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/BaseRandomGeneratorTest.java b/src/test/java/org/apache/commons/math4/random/BaseRandomGeneratorTest.java
new file mode 100644
index 0000000..40ab9b5
--- /dev/null
+++ b/src/test/java/org/apache/commons/math4/random/BaseRandomGeneratorTest.java
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.math4.random;
+
+import java.util.Random;
+
+import org.apache.commons.math4.exception.OutOfRangeException;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Tests for the generic implementations of the methods defined in the
+ * {@link BaseRandomGenerator} class, using the standard {@link Random}
+ * class as the source of randomness.
+ */
+public class BaseRandomGeneratorTest extends RandomGeneratorAbstractTest {
+ /** To simplify testing of additional utility methods. */
+ protected BaseRandomGenerator baseRandomGenerator;
+
+ @Before
+ public void setUp() {
+ baseRandomGenerator = (BaseRandomGenerator) generator;
+ }
+
+ @Override
+ protected RandomGenerator makeGenerator() {
+ final RandomGenerator generator = new TestGenerator();
+ generator.setSeed(1000);
+ return generator;
+ }
+
+ @Test(expected=OutOfRangeException.class)
+ public void testNextBytesPrecondition1() {
+ final int len = 3;
+ final byte[] b = new byte[len];
+ baseRandomGenerator.nextBytes(b, -1, 1);
+ }
+
+ @Test(expected=OutOfRangeException.class)
+ public void testNextBytesPrecondition2() {
+ final int len = 3;
+ final byte[] b = new byte[len];
+ baseRandomGenerator.nextBytes(b, len, 0);
+ }
+
+ @Test(expected=OutOfRangeException.class)
+ public void testNextBytesPrecondition3() {
+ final int len = 3;
+ final byte[] b = new byte[len];
+ baseRandomGenerator.nextBytes(b, 0, 0);
+ }
+
+ @Test(expected=OutOfRangeException.class)
+ public void testNextBytesPrecondition4() {
+ final int len = 3;
+ final byte[] b = new byte[len];
+ baseRandomGenerator.nextBytes(b, 0, len + 1);
+ }
+
+ @Test
+ public void testNextBytesSubArray() {
+ final int size = 123;
+ final int insert = 72;
+ final int len = 37;
+
+ final byte[] buffer = new byte[size];
+ baseRandomGenerator.nextBytes(buffer, insert, len);
+
+ final byte[] bufferCopy = buffer.clone();
+ baseRandomGenerator.nextBytes(buffer, insert, len);
+
+ for (int i = 0; i < insert; i++) {
+ Assert.assertEquals(bufferCopy[i], buffer[i]);
+ }
+ final int maxInsert = insert + len;
+ for (int i = insert; i < maxInsert; i++) {
+ Assert.assertNotEquals(bufferCopy[i], buffer[i]);
+ }
+ for (int i = maxInsert; i < size; i++) {
+ Assert.assertEquals(bufferCopy[i], buffer[i]);
+ }
+ }
+
+ /**
+ * Test RNG delegating to {@link Random}.
+ */
+ private static class TestGenerator extends BaseRandomGenerator {
+ /** Delegate. */
+ private Random random = new Random();
+
+ @Override
+ public void setSeed(int seed) {
+ random.setSeed(seed);
+ clear();
+ }
+
+ @Override
+ public void setSeed(int[] seed) {
+ random.setSeed(seed[0]);
+ }
+
+ @Override
+ public void setSeed(long seed) {
+ random.setSeed((int) seed);
+ }
+
+ @Override
+ public int nextInt() {
+ // Delegate.
+ return random.nextInt();
+ }
+ }
+}
[06/18] [math] MATH-1307
Posted by er...@apache.org.
MATH-1307
Extends new base class.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/ffae3bdb
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/ffae3bdb
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/ffae3bdb
Branch: refs/heads/master
Commit: ffae3bdbb6740e336a223a82a3dd545c24cde7b5
Parents: 921d0d6
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 16:51:31 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 16:51:31 2015 +0100
----------------------------------------------------------------------
.../commons/math4/random/AbstractWell.java | 10 ++++-----
.../apache/commons/math4/random/Well1024a.java | 22 ++++++++++----------
.../apache/commons/math4/random/Well19937a.java | 6 ++----
.../apache/commons/math4/random/Well19937c.java | 6 ++----
.../apache/commons/math4/random/Well44497a.java | 6 ++----
.../apache/commons/math4/random/Well44497b.java | 6 ++----
.../apache/commons/math4/random/Well512a.java | 6 ++----
.../commons/math4/random/Well1024aTest.java | 2 +-
.../commons/math4/random/Well19937aTest.java | 2 +-
.../commons/math4/random/Well19937cTest.java | 2 +-
.../commons/math4/random/Well44497aTest.java | 2 +-
.../commons/math4/random/Well44497bTest.java | 2 +-
.../commons/math4/random/Well512aTest.java | 2 +-
13 files changed, 31 insertions(+), 43 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/main/java/org/apache/commons/math4/random/AbstractWell.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/AbstractWell.java b/src/main/java/org/apache/commons/math4/random/AbstractWell.java
index 7251bcd..da222fd 100644
--- a/src/main/java/org/apache/commons/math4/random/AbstractWell.java
+++ b/src/main/java/org/apache/commons/math4/random/AbstractWell.java
@@ -34,10 +34,12 @@ import org.apache.commons.math4.util.FastMath;
* @see <a href="http://www.iro.umontreal.ca/~panneton/WELLRNG.html">WELL Random number generator</a>
* @since 2.2
*/
-public abstract class AbstractWell extends BitsStreamGenerator implements Serializable {
+public abstract class AbstractWell
+ extends BaseRandomGenerator
+ implements Serializable {
/** Serializable version identifier. */
- private static final long serialVersionUID = 20150223L;
+ private static final long serialVersionUID = 20150228L;
/** Current index in the bytes pool. */
protected int index;
@@ -133,10 +135,6 @@ public abstract class AbstractWell extends BitsStreamGenerator implements Serial
setSeed(new int[] { (int) (seed >>> 32), (int) (seed & 0xffffffffl) });
}
- /** {@inheritDoc} */
- @Override
- protected abstract int next(final int bits);
-
/** Calculate the number of 32-bits blocks.
* @param k number of bits in the pool (not necessarily a multiple of 32)
* @return the number of 32-bits blocks
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/main/java/org/apache/commons/math4/random/Well1024a.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/Well1024a.java b/src/main/java/org/apache/commons/math4/random/Well1024a.java
index 39153f2..ff1de07 100644
--- a/src/main/java/org/apache/commons/math4/random/Well1024a.java
+++ b/src/main/java/org/apache/commons/math4/random/Well1024a.java
@@ -16,15 +16,18 @@
*/
package org.apache.commons.math4.random;
-/** This class implements the WELL1024a pseudo-random number generator
+/**
+ * This class implements the WELL1024a pseudo-random number generator
* from François Panneton, Pierre L'Ecuyer and Makoto Matsumoto.
+ *
* <p>
* This generator is described in a paper by François Panneton,
- * Pierre L'Ecuyer and Makoto Matsumoto <a
- * href="http://www.iro.umontreal.ca/~lecuyer/myftp/papers/wellrng.pdf">Improved
- * Long-Period Generators Based on Linear Recurrences Modulo 2</a> ACM
- * Transactions on Mathematical Software, 32, 1 (2006). The errata for the paper
- * are in <a href="http://www.iro.umontreal.ca/~lecuyer/myftp/papers/wellrng-errata.txt">wellrng-errata.txt</a>.</p>
+ * Pierre L'Ecuyer and Makoto Matsumoto
+ * <a href="http://www.iro.umontreal.ca/~lecuyer/myftp/papers/wellrng.pdf">
+ * Improved Long-Period Generators Based on Linear Recurrences Modulo 2</a>
+ * ACM Transactions on Mathematical Software, 32, 1 (2006). The errata for the paper
+ * are in <a href="http://www.iro.umontreal.ca/~lecuyer/myftp/papers/wellrng-errata.txt">wellrng-errata.txt</a>.
+ * </p>
*
* @see <a href="http://www.iro.umontreal.ca/~panneton/WELLRNG.html">WELL Random number generator</a>
* @since 2.2
@@ -81,8 +84,7 @@ public class Well1024a extends AbstractWell {
/** {@inheritDoc} */
@Override
- protected int next(final int bits) {
-
+ public int nextInt() {
final int indexRm1 = TABLE.getIndexPred(index);
final int v0 = v[index];
@@ -100,8 +102,6 @@ public class Well1024a extends AbstractWell {
v[indexRm1] = z4;
index = indexRm1;
- return z4 >>> (32 - bits);
-
+ return z4;
}
-
}
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/main/java/org/apache/commons/math4/random/Well19937a.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/Well19937a.java b/src/main/java/org/apache/commons/math4/random/Well19937a.java
index 856a42d..d4b877a 100644
--- a/src/main/java/org/apache/commons/math4/random/Well19937a.java
+++ b/src/main/java/org/apache/commons/math4/random/Well19937a.java
@@ -82,7 +82,7 @@ public class Well19937a extends AbstractWell {
/** {@inheritDoc} */
@Override
- protected int next(final int bits) {
+ public int nextInt() {
final int indexRm1 = TABLE.getIndexPred(index);
final int indexRm2 = TABLE.getIndexPred2(index);
@@ -103,8 +103,6 @@ public class Well19937a extends AbstractWell {
v[indexRm2] &= 0x80000000;
index = indexRm1;
- return z4 >>> (32 - bits);
-
+ return z4;
}
-
}
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/main/java/org/apache/commons/math4/random/Well19937c.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/Well19937c.java b/src/main/java/org/apache/commons/math4/random/Well19937c.java
index 1ff77c7..c15350d 100644
--- a/src/main/java/org/apache/commons/math4/random/Well19937c.java
+++ b/src/main/java/org/apache/commons/math4/random/Well19937c.java
@@ -82,7 +82,7 @@ public class Well19937c extends AbstractWell {
/** {@inheritDoc} */
@Override
- protected int next(final int bits) {
+ public int nextInt() {
final int indexRm1 = TABLE.getIndexPred(index);
final int indexRm2 = TABLE.getIndexPred2(index);
@@ -108,8 +108,6 @@ public class Well19937c extends AbstractWell {
z4 ^= (z4 << 7) & 0xe46e1700;
z4 ^= (z4 << 15) & 0x9b868000;
- return z4 >>> (32 - bits);
-
+ return z4;
}
-
}
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/main/java/org/apache/commons/math4/random/Well44497a.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/Well44497a.java b/src/main/java/org/apache/commons/math4/random/Well44497a.java
index 1c7765f..667d458 100644
--- a/src/main/java/org/apache/commons/math4/random/Well44497a.java
+++ b/src/main/java/org/apache/commons/math4/random/Well44497a.java
@@ -81,7 +81,7 @@ public class Well44497a extends AbstractWell {
/** {@inheritDoc} */
@Override
- protected int next(final int bits) {
+ public int nextInt() {
final int indexRm1 = TABLE.getIndexPred(index);
final int indexRm2 = TABLE.getIndexPred2(index);
@@ -105,8 +105,6 @@ public class Well44497a extends AbstractWell {
v[indexRm2] &= 0xFFFF8000;
index = indexRm1;
- return z4 >>> (32 - bits);
-
+ return z4;
}
-
}
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/main/java/org/apache/commons/math4/random/Well44497b.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/Well44497b.java b/src/main/java/org/apache/commons/math4/random/Well44497b.java
index 551da47..91c506f 100644
--- a/src/main/java/org/apache/commons/math4/random/Well44497b.java
+++ b/src/main/java/org/apache/commons/math4/random/Well44497b.java
@@ -81,7 +81,7 @@ public class Well44497b extends AbstractWell {
/** {@inheritDoc} */
@Override
- protected int next(final int bits) {
+ public int nextInt() {
// compute raw value given by WELL44497a generator
// which is NOT maximally-equidistributed
@@ -112,8 +112,6 @@ public class Well44497b extends AbstractWell {
z4 ^= (z4 << 7) & 0x93dd1400;
z4 ^= (z4 << 15) & 0xfa118000;
- return z4 >>> (32 - bits);
-
+ return z4;
}
-
}
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/main/java/org/apache/commons/math4/random/Well512a.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/Well512a.java b/src/main/java/org/apache/commons/math4/random/Well512a.java
index 391bb2f..55fc6db 100644
--- a/src/main/java/org/apache/commons/math4/random/Well512a.java
+++ b/src/main/java/org/apache/commons/math4/random/Well512a.java
@@ -82,7 +82,7 @@ public class Well512a extends AbstractWell {
/** {@inheritDoc} */
@Override
- protected int next(final int bits) {
+ public int nextInt() {
final int indexRm1 = TABLE.getIndexPred(index);
@@ -101,8 +101,6 @@ public class Well512a extends AbstractWell {
v[indexRm1] = z4;
index = indexRm1;
- return z4 >>> (32 - bits);
-
+ return z4;
}
-
}
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/test/java/org/apache/commons/math4/random/Well1024aTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/Well1024aTest.java b/src/test/java/org/apache/commons/math4/random/Well1024aTest.java
index d5e0e67..242c24d 100644
--- a/src/test/java/org/apache/commons/math4/random/Well1024aTest.java
+++ b/src/test/java/org/apache/commons/math4/random/Well1024aTest.java
@@ -21,7 +21,7 @@ import org.apache.commons.math4.random.Well1024a;
import org.junit.Assert;
import org.junit.Test;
-public class Well1024aTest extends RandomGeneratorAbstractTest {
+public class Well1024aTest extends BaseRandomGeneratorTest {
@Override
protected RandomGenerator makeGenerator() {
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/test/java/org/apache/commons/math4/random/Well19937aTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/Well19937aTest.java b/src/test/java/org/apache/commons/math4/random/Well19937aTest.java
index 97f05f4..00ff92b 100644
--- a/src/test/java/org/apache/commons/math4/random/Well19937aTest.java
+++ b/src/test/java/org/apache/commons/math4/random/Well19937aTest.java
@@ -21,7 +21,7 @@ import org.apache.commons.math4.random.Well19937a;
import org.junit.Assert;
import org.junit.Test;
-public class Well19937aTest extends RandomGeneratorAbstractTest {
+public class Well19937aTest extends BaseRandomGeneratorTest {
@Override
public RandomGenerator makeGenerator() {
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/test/java/org/apache/commons/math4/random/Well19937cTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/Well19937cTest.java b/src/test/java/org/apache/commons/math4/random/Well19937cTest.java
index cfffcd1..a97cbcc 100644
--- a/src/test/java/org/apache/commons/math4/random/Well19937cTest.java
+++ b/src/test/java/org/apache/commons/math4/random/Well19937cTest.java
@@ -21,7 +21,7 @@ import org.apache.commons.math4.random.Well19937c;
import org.junit.Assert;
import org.junit.Test;
-public class Well19937cTest extends RandomGeneratorAbstractTest {
+public class Well19937cTest extends BaseRandomGeneratorTest {
@Override
public RandomGenerator makeGenerator() {
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/test/java/org/apache/commons/math4/random/Well44497aTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/Well44497aTest.java b/src/test/java/org/apache/commons/math4/random/Well44497aTest.java
index e6dc96f..426f3cd 100644
--- a/src/test/java/org/apache/commons/math4/random/Well44497aTest.java
+++ b/src/test/java/org/apache/commons/math4/random/Well44497aTest.java
@@ -21,7 +21,7 @@ import org.apache.commons.math4.random.Well44497a;
import org.junit.Assert;
import org.junit.Test;
-public class Well44497aTest extends RandomGeneratorAbstractTest {
+public class Well44497aTest extends BaseRandomGeneratorTest {
@Override
public RandomGenerator makeGenerator() {
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/test/java/org/apache/commons/math4/random/Well44497bTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/Well44497bTest.java b/src/test/java/org/apache/commons/math4/random/Well44497bTest.java
index 7f3dee1..6ce8d79 100644
--- a/src/test/java/org/apache/commons/math4/random/Well44497bTest.java
+++ b/src/test/java/org/apache/commons/math4/random/Well44497bTest.java
@@ -21,7 +21,7 @@ import org.apache.commons.math4.random.Well44497b;
import org.junit.Assert;
import org.junit.Test;
-public class Well44497bTest extends RandomGeneratorAbstractTest {
+public class Well44497bTest extends BaseRandomGeneratorTest {
@Override
public RandomGenerator makeGenerator() {
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ffae3bdb/src/test/java/org/apache/commons/math4/random/Well512aTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/Well512aTest.java b/src/test/java/org/apache/commons/math4/random/Well512aTest.java
index 192d7d5..3f63688 100644
--- a/src/test/java/org/apache/commons/math4/random/Well512aTest.java
+++ b/src/test/java/org/apache/commons/math4/random/Well512aTest.java
@@ -21,7 +21,7 @@ import org.apache.commons.math4.random.Well512a;
import org.junit.Assert;
import org.junit.Test;
-public class Well512aTest extends RandomGeneratorAbstractTest {
+public class Well512aTest extends BaseRandomGeneratorTest {
@Override
public RandomGenerator makeGenerator() {
[13/18] [math] MATH-1307
Posted by er...@apache.org.
MATH-1307
Use new class.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/d8e2f892
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/d8e2f892
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/d8e2f892
Branch: refs/heads/master
Commit: d8e2f892e48423fda73204be3ec239d282e71c65
Parents: ca01fdf
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 17:55:00 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 17:55:00 2015 +0100
----------------------------------------------------------------------
.../distribution/ZipfDistributionTest.java | 37 ++++++++++++--------
1 file changed, 22 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/d8e2f892/src/test/java/org/apache/commons/math4/distribution/ZipfDistributionTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/distribution/ZipfDistributionTest.java b/src/test/java/org/apache/commons/math4/distribution/ZipfDistributionTest.java
index eeae81f..1a71626 100644
--- a/src/test/java/org/apache/commons/math4/distribution/ZipfDistributionTest.java
+++ b/src/test/java/org/apache/commons/math4/distribution/ZipfDistributionTest.java
@@ -20,7 +20,7 @@ package org.apache.commons.math4.distribution;
import org.apache.commons.math4.TestUtils;
import org.apache.commons.math4.distribution.ZipfDistribution.ZipfRejectionInversionSampler;
import org.apache.commons.math4.exception.NotStrictlyPositiveException;
-import org.apache.commons.math4.random.AbstractRandomGenerator;
+import org.apache.commons.math4.random.BaseRandomGenerator;
import org.apache.commons.math4.random.RandomGenerator;
import org.apache.commons.math4.random.Well1024a;
import org.apache.commons.math4.util.FastMath;
@@ -215,20 +215,27 @@ public class ZipfDistributionTest extends IntegerDistributionAbstractTest {
long start = System.currentTimeMillis();
final int[] randomNumberCounter = new int[1];
- RandomGenerator randomGenerator = new AbstractRandomGenerator() {
-
- private final RandomGenerator r = new Well1024a(0L);
-
- @Override
- public void setSeed(long seed) {
- }
-
- @Override
- public double nextDouble() {
- randomNumberCounter[0]+=1;
- return r.nextDouble();
- }
- };
+ RandomGenerator randomGenerator = new BaseRandomGenerator() {
+ private final RandomGenerator r = new Well1024a(0L);
+
+ @Override
+ public void setSeed(long s) {
+ r.setSeed(s);
+ }
+ @Override
+ public void setSeed(int s) {
+ r.setSeed(s);
+ }
+ @Override
+ public void setSeed(int[] s) {
+ r.setSeed(s);
+ }
+ @Override
+ public int nextInt() {
+ randomNumberCounter[0] += 1;
+ return r.nextInt();
+ }
+ };
final ZipfDistribution distribution = new ZipfDistribution(randomGenerator, numPoints, exponent);
for (int i = 0; i < numGeneratedSamples; ++i) {
[09/18] [math] MATH-1309
Posted by er...@apache.org.
MATH-1309
Not calling public "setSeed" from a constructor.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/581b474f
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/581b474f
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/581b474f
Branch: refs/heads/master
Commit: 581b474f4b194731eb9cb22cdde7330a1ec81c3b
Parents: e34f50d
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 17:05:13 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 17:32:34 2015 +0100
----------------------------------------------------------------------
.../commons/math4/random/AbstractWell.java | 34 ++++++++++++++------
1 file changed, 25 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/581b474f/src/main/java/org/apache/commons/math4/random/AbstractWell.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/AbstractWell.java b/src/main/java/org/apache/commons/math4/random/AbstractWell.java
index b2d61a3..88cb2bb 100644
--- a/src/main/java/org/apache/commons/math4/random/AbstractWell.java
+++ b/src/main/java/org/apache/commons/math4/random/AbstractWell.java
@@ -79,7 +79,7 @@ public abstract class AbstractWell
index = 0;
// Initialize the pool content.
- setSeed(seed);
+ setSeedInternal(seed);
}
/**
@@ -92,6 +92,25 @@ public abstract class AbstractWell
this(k, new int[] { (int) (seed >>> 32), (int) (seed & 0xffffffffl) });
}
+
+ /** {@inheritDoc} */
+ @Override
+ public void setSeed(int seed) {
+ setSeedInternal(seed);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void setSeed(int[] seed) {
+ setSeedInternal(seed);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void setSeed(long seed) {
+ setSeedInternal(seed);
+ }
+
/**
* Reinitialize the generator as if just built with the given int seed.
*
@@ -100,9 +119,8 @@ public abstract class AbstractWell
*
* @param seed Seed (32 bits integer).
*/
- @Override
- public void setSeed(final int seed) {
- setSeed(new int[] { seed });
+ private void setSeedInternal(final int seed) {
+ setSeedInternal(new int[] { seed });
}
/**
@@ -114,8 +132,7 @@ public abstract class AbstractWell
* @param seed Seed (32 bits integers array). If null the seed of the generator
* will be the system time plus the system identity hash code of the instance.
*/
- @Override
- public void setSeed(final int[] seed) {
+ private void setSeedInternal(final int[] seed) {
if (seed == null) {
setSeed(System.currentTimeMillis() + System.identityHashCode(this));
return;
@@ -142,9 +159,8 @@ public abstract class AbstractWell
*
* @param seed Seed (64 bits integer).
*/
- @Override
- public void setSeed(final long seed) {
- setSeed(new int[] { (int) (seed >>> 32), (int) (seed & 0xffffffffl) });
+ private void setSeedInternal(final long seed) {
+ setSeedInternal(new int[] { (int) (seed >>> 32), (int) (seed & 0xffffffffl) });
}
/**
[07/18] [math] Javadoc.
Posted by er...@apache.org.
Javadoc.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/f9e72000
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/f9e72000
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/f9e72000
Branch: refs/heads/master
Commit: f9e72000a17eb9d524b38a9f2c982c037dd94891
Parents: ffae3bd
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 17:01:21 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 17:01:21 2015 +0100
----------------------------------------------------------------------
.../commons/math4/random/AbstractWell.java | 90 +++++++++++---------
.../commons/math4/random/MersenneTwister.java | 21 +++--
2 files changed, 64 insertions(+), 47 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/f9e72000/src/main/java/org/apache/commons/math4/random/AbstractWell.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/AbstractWell.java b/src/main/java/org/apache/commons/math4/random/AbstractWell.java
index da222fd..a389d58 100644
--- a/src/main/java/org/apache/commons/math4/random/AbstractWell.java
+++ b/src/main/java/org/apache/commons/math4/random/AbstractWell.java
@@ -37,73 +37,82 @@ import org.apache.commons.math4.util.FastMath;
public abstract class AbstractWell
extends BaseRandomGenerator
implements Serializable {
-
/** Serializable version identifier. */
private static final long serialVersionUID = 20150228L;
-
/** Current index in the bytes pool. */
protected int index;
-
/** Bytes pool. */
protected final int[] v;
- /** Creates a new random number generator.
+ /**
+ * Creates a new random number generator.
+ *
* <p>The instance is initialized using the current time plus the
* system identity hash code of this instance as the seed.</p>
- * @param k number of bits in the pool (not necessarily a multiple of 32)
+ *
+ * @param k number of bits in the pool (not necessarily a multiple of 32).
*/
protected AbstractWell(final int k) {
this(k, null);
}
- /** Creates a new random number generator using a single int seed.
- * @param k number of bits in the pool (not necessarily a multiple of 32)
- * @param seed the initial seed (32 bits integer)
+ /**
+ * Creates a new random number generator using a single int seed.
+ *
+ * @param k number of bits in the pool (not necessarily a multiple of 32).
+ * @param seed the initial seed (32 bits integer).
*/
protected AbstractWell(final int k, final int seed) {
this(k, new int[] { seed });
}
- /** Creates a new random number generator using an int array seed.
- * @param k number of bits in the pool (not necessarily a multiple of 32)
+ /**
+ * Creates a new random number generator using an int array seed.
+ *
+ * @param k number of bits in the pool (not necessarily a multiple of 32).
* @param seed the initial seed (32 bits integers array), if null
- * the seed of the generator will be related to the current time
+ * the seed of the generator will be related to the current time.
*/
protected AbstractWell(final int k, final int[] seed) {
-
final int r = calculateBlockCount(k);
- this.v = new int[r];
- this.index = 0;
+ v = new int[r];
+ index = 0;
- // initialize the pool content
+ // Initialize the pool content.
setSeed(seed);
-
}
- /** Creates a new random number generator using a single long seed.
- * @param k number of bits in the pool (not necessarily a multiple of 32)
- * @param seed the initial seed (64 bits integer)
+ /**
+ * Creates a new random number generator using a single long seed.
+ *
+ * @param k number of bits in the pool (not necessarily a multiple of 32).
+ * @param seed the initial seed (64 bits integer).
*/
protected AbstractWell(final int k, final long seed) {
this(k, new int[] { (int) (seed >>> 32), (int) (seed & 0xffffffffl) });
}
- /** Reinitialize the generator as if just built with the given int seed.
- * <p>The state of the generator is exactly the same as a new
- * generator built with the same seed.</p>
- * @param seed the initial seed (32 bits integer)
+ /**
+ * Reinitialize the generator as if just built with the given int seed.
+ *
+ * <p>The state of the generator is exactly the same as a new generator
+ * built with the same seed.</p>
+ *
+ * @param seed Seed (32 bits integer).
*/
@Override
public void setSeed(final int seed) {
setSeed(new int[] { seed });
}
- /** Reinitialize the generator as if just built with the given int array seed.
- * <p>The state of the generator is exactly the same as a new
- * generator built with the same seed.</p>
- * @param seed the initial seed (32 bits integers array). If null
- * the seed of the generator will be the system time plus the system identity
- * hash code of the instance.
+ /**
+ * Reinitialize the generator as if just built with the given int array seed.
+ *
+ * <p>The state of the generator is exactly the same as a new generator built
+ * with the same seed.</p>
+ *
+ * @param seed Seed (32 bits integers array). If null the seed of the generator
+ * will be the system time plus the system identity hash code of the instance.
*/
@Override
public void setSeed(final int[] seed) {
@@ -125,19 +134,24 @@ public abstract class AbstractWell
clear(); // Clear normal deviate cache
}
- /** Reinitialize the generator as if just built with the given long seed.
- * <p>The state of the generator is exactly the same as a new
- * generator built with the same seed.</p>
- * @param seed the initial seed (64 bits integer)
+ /**
+ * Reinitialize the generator as if just built with the given long seed.
+ *
+ * <p>The state of the generator is exactly the same as a new generator built
+ * with the same seed.</p>
+ *
+ * @param seed Seed (64 bits integer).
*/
@Override
public void setSeed(final long seed) {
setSeed(new int[] { (int) (seed >>> 32), (int) (seed & 0xffffffffl) });
}
- /** Calculate the number of 32-bits blocks.
- * @param k number of bits in the pool (not necessarily a multiple of 32)
- * @return the number of 32-bits blocks
+ /**
+ * Calculate the number of 32-bits blocks.
+ *
+ * @param k Number of bits in the pool (not necessarily a multiple of 32).
+ * @return the number of 32-bits blocks.
*/
private static int calculateBlockCount(final int k) {
// the bits pool contains k bits, k = r w - p where r is the number
@@ -149,8 +163,8 @@ public abstract class AbstractWell
}
/**
- * Inner class used to store the indirection index table which is fixed for a given type of WELL class
- * of pseudo-random number generator.
+ * Inner class used to store the indirection index table which is fixed for a given
+ * type of WELL class of pseudo-random number generator.
*
* @since 4.0
*/
http://git-wip-us.apache.org/repos/asf/commons-math/blob/f9e72000/src/main/java/org/apache/commons/math4/random/MersenneTwister.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/MersenneTwister.java b/src/main/java/org/apache/commons/math4/random/MersenneTwister.java
index d8caaa7..709c343 100644
--- a/src/main/java/org/apache/commons/math4/random/MersenneTwister.java
+++ b/src/main/java/org/apache/commons/math4/random/MersenneTwister.java
@@ -222,21 +222,24 @@ public class MersenneTwister
setSeed(new int[] { (int) (seed >>> 32), (int) (seed & 0xffffffffl) });
}
- /** Generate next pseudorandom number.
- * <p>This method is the core generation algorithm. It is used by all the
- * public generation methods for the various primitive types {@link
- * #nextBoolean()}, {@link #nextBytes(byte[])}, {@link #nextDouble()},
+ /**
+ * Generates the next pseudorandom number.
+ *
+ * <p>
+ * This method is the core generation algorithm. It is used by all the
+ * public generation methods for the various primitive types
+ * {@link #nextBoolean()}, {@link #nextBytes(byte[])}, {@link #nextDouble()},
* {@link #nextFloat()}, {@link #nextGaussian()} and {@link #nextLong()}.
* </p>
- * @param bits number of random bits to produce
- * @return random bits generated
+ *
+ * @param bits Number of random bits to produce.
+ * @return the random bits generated.
*/
@Override
public int nextInt() {
-
int y;
- if (mti >= N) { // generate N words at one time
+ if (mti >= N) { // Generate N words at one time.
int mtNext = mt[0];
for (int k = 0; k < N - M; ++k) {
int mtCurr = mtNext;
@@ -258,7 +261,7 @@ public class MersenneTwister
y = mt[mti++];
- // tempering
+ // Tempering.
y ^= y >>> 11;
y ^= (y << 7) & 0x9d2c5680;
y ^= (y << 15) & 0xefc60000;
[15/18] [math] Javadoc.
Posted by er...@apache.org.
Javadoc.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/b95df5ad
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/b95df5ad
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/b95df5ad
Branch: refs/heads/master
Commit: b95df5ad1ecfe158f4acbd6e62c8429e35969ae2
Parents: aaabfe8
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 18:07:23 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 18:07:23 2015 +0100
----------------------------------------------------------------------
.../java/org/apache/commons/math4/random/RandomDataGenerator.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/b95df5ad/src/main/java/org/apache/commons/math4/random/RandomDataGenerator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/RandomDataGenerator.java b/src/main/java/org/apache/commons/math4/random/RandomDataGenerator.java
index 7488a5f..eb8415d 100644
--- a/src/main/java/org/apache/commons/math4/random/RandomDataGenerator.java
+++ b/src/main/java/org/apache/commons/math4/random/RandomDataGenerator.java
@@ -55,7 +55,7 @@ import org.apache.commons.math4.util.MathArrays;
* <code>RandomGenerator</code> is provided in the constructor, the default is
* to use a {@link Well19937c} generator. To plug in a different
* implementation, either implement <code>RandomGenerator</code> directly or
- * extend {@link AbstractRandomGenerator}.
+ * extend {@link BaseRandomGenerator}.
* <p>
* Supports reseeding the underlying pseudo-random number generator (PRNG). The
* <code>SecurityProvider</code> and <code>Algorithm</code> used by the
[05/18] [math] MATH-1307
Posted by er...@apache.org.
MATH-1307
Extends new base class.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/921d0d60
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/921d0d60
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/921d0d60
Branch: refs/heads/master
Commit: 921d0d60d40d8fbda29f14a64e848c70026ea32e
Parents: 87497c7
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 16:49:55 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 16:49:55 2015 +0100
----------------------------------------------------------------------
.../java/org/apache/commons/math4/random/ISAACRandom.java | 10 ++++++----
.../java/org/apache/commons/math4/random/ISAACTest.java | 2 +-
2 files changed, 7 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/921d0d60/src/main/java/org/apache/commons/math4/random/ISAACRandom.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/ISAACRandom.java b/src/main/java/org/apache/commons/math4/random/ISAACRandom.java
index ed07ddf..b9c0e36 100644
--- a/src/main/java/org/apache/commons/math4/random/ISAACRandom.java
+++ b/src/main/java/org/apache/commons/math4/random/ISAACRandom.java
@@ -41,9 +41,11 @@ import org.apache.commons.math4.util.FastMath;
*
* @since 3.0
*/
-public class ISAACRandom extends BitsStreamGenerator implements Serializable {
+public class ISAACRandom
+ extends BaseRandomGenerator
+ implements Serializable {
/** Serializable version identifier */
- private static final long serialVersionUID = 7288197941165002400L;
+ private static final long serialVersionUID = 20151227L;
/** Log of size of rsl[] and mem[] */
private static final int SIZE_L = 8;
/** Size of rsl[] and mem[] */
@@ -138,12 +140,12 @@ public class ISAACRandom extends BitsStreamGenerator implements Serializable {
/** {@inheritDoc} */
@Override
- protected int next(int bits) {
+ public int nextInt() {
if (count < 0) {
isaac();
count = SIZE - 1;
}
- return rsl[count--] >>> 32 - bits;
+ return rsl[count--];
}
/** Generate 256 results */
http://git-wip-us.apache.org/repos/asf/commons-math/blob/921d0d60/src/test/java/org/apache/commons/math4/random/ISAACTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/ISAACTest.java b/src/test/java/org/apache/commons/math4/random/ISAACTest.java
index f8e7e65..6db04ba 100644
--- a/src/test/java/org/apache/commons/math4/random/ISAACTest.java
+++ b/src/test/java/org/apache/commons/math4/random/ISAACTest.java
@@ -22,7 +22,7 @@ import org.apache.commons.math4.random.RandomGenerator;
import org.junit.Assert;
import org.junit.Test;
-public final class ISAACTest extends RandomGeneratorAbstractTest {
+public final class ISAACTest extends BaseRandomGeneratorTest {
@Override
protected RandomGenerator makeGenerator() {
[02/18] [math] Javadoc.
Posted by er...@apache.org.
Javadoc.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/2a8061f4
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/2a8061f4
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/2a8061f4
Branch: refs/heads/master
Commit: 2a8061f4ad8903a9c6d46b404973bdbd84385d39
Parents: 4cbb388
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 16:46:31 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 16:46:31 2015 +0100
----------------------------------------------------------------------
.../apache/commons/math4/random/BaseRandomGenerator.java | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/2a8061f4/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java b/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
index a78fd38..2d78005 100644
--- a/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
+++ b/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
@@ -175,8 +175,7 @@ public abstract class BaseRandomGenerator
* Generates random bytes and places them into a user-supplied array.
*
* <p>
- * The array is filled with bytes extracted from random integers generated
- * using {@link #nextInt()}.
+ * The array is filled with bytes extracted from random integers.
* This implies that the number of random bytes generated may be larger than
* the length of the byte array.
* </p>
@@ -192,8 +191,7 @@ public abstract class BaseRandomGenerator
* Generates random bytes and places them into a user-supplied array.
*
* <p>
- * The array is filled with bytes extracted from random integers generated
- * using {@link #nextInt()}.
+ * The array is filled with bytes extracted from random integers.
* This implies that the number of random bytes generated may be larger than
* the length of the byte array.
* </p>
@@ -224,8 +222,7 @@ public abstract class BaseRandomGenerator
* Generates random bytes and places them into a user-supplied array.
*
* <p>
- * The array is filled with bytes extracted from random integers generated
- * using {@link #nextInt()}.
+ * The array is filled with bytes extracted from random integers.
* This implies that the number of random bytes generated may be larger than
* the length of the byte array.
* </p>
Re: [Math] About the refactoring of RNGs
Posted by Luc Maisonobe <lu...@spaceroots.org>.
hi all,
Le 29/12/2015 18:32, Phil Steitz a écrit :
>
>
>> On Dec 29, 2015, at 8:41 AM, Gilles <gi...@harfang.homelinux.org>
>> wrote:
>>
>>> On Mon, 28 Dec 2015 20:33:24 -0700, Phil Steitz wrote:
>>>> On 12/28/15 8:08 PM, Gilles wrote:
>>>>> 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.
>>>
>>> No. Not sufficient. What you have done is changed the internal
>>> implementation of all of the Bitstream generators. I am not
>>> convinced that you have not broken anything. I will have to do
>>> the testing myself. I see no point in fiddling with the
>>> internals of this code that has had a lot of eyeballs and testing
>>> on it. I was not personally looking forward to researching the
>>> algorithms to make sure any invariants may be broken by these
>>> changes; but I am now going to have to do this. I have to ask
>>> why. Please at some point read [1], especially the sections on
>>> "Avoid Flexibility Syndrom" and "Value Laziness as a Virtue."
>>> Gratuitous refactoring drains community energy.
>>
>> It seems that again you don't read what I write.[1] Hence the above
>> paragraph is spreading F -> "I am not convinced that you have not
>> broken anything." U -> "I will have to do the testing myself." D ->
>> "I see no point in fiddling [...]" .
>>
>> Or maybe I have to rant about email communication. Please reread
>> the thread to fully appreciate that you could have shared your
>> doubts among the opinions which you gave about some of my
>> questions. When the reply answers to only some of the direct
>> questions, the OP is legitimately tempted to assume that the
>> non-comment is akin to "I don't care" (as in "left to the judgment
>> of the one who does the job").
>>
>> As is often the case, you can dislike my ideas of improvements
>> (ideas that come on the basis of the information provided by what
>> the code does) but I don't appreciate the use of the word
>> "gratuitous", given that I clearly stated the purpose of making
>> explicit what the code actually does (that is, *generating* 32
>> random bits and not the number of bits passed as a parameter to
>> "next(int)"). So, the code is now self-documenting; it is a small
>> change, to be sure, but hardly gratuitous.
>
> I did not respond to the original question about eliminating
> next(int) because I was not (still am not) expert in the internals of
> how the generators work and how the generated ints-of-bits are
> transformed to make doubles, gaussians, etc. I was hoping someone
> who was expert would chime in and say either "don't do that" or
> "that's ok".
I intend to look at these changes, but concentrated on the last
things I wanted to be included in 3.6 (another mail will come
about this later this evening).
I am not an expert either on PRNG, but did work on some of the
implementations we have, so maybe I will be able to give an
opinion.
One thing we coud do is set up a branch for such an experimentation.
Branches in Git are simple. The work could then be merged back
to master once it has stabilized. It would also be easy for any
developer to switch back and forth between this feature branch
and master to get convinced there are no hidden problems.
> That did not happen before you committed the changes.
> That obligates us to review them. The tests cover only nextInt so
> we need to convince ourselves that the transformations (which have
> all changed) are still correct. That is in my mind just extra work
> generated for the community. I would rather see our cycles go toward
> getting 3.6 out.
So let the dust settle down for a few days, I would really like
to finalize 3.6 too.
best regards,
Luc
>
> I stand by my assertion that the rebasing to eliminate next(bits)
> adds nothing. If you can demonstrate via benchmarks this it is
> faster, I will retract that statement.
>
> I know we disagree fundamentally on the priorities of [math]. To me,
> stability, correctness and ease of use (including ease of upgrades)
> are paramount. That means you don't make implementation changes to
> hardened code without a good reason and you limit incompatible change
> to what is necessary to deliver practical benefit to users in new
> features or bug fixes. I was +1 to dropping AbstractRandomGenerator
> and just leaving the bitstream generators in place. That is a
> simplifying change.
>>
>> I actually did go some way towards "Avoiding [a] Flexibility
>> Syndrom" that *was* present in a seeming flexibility of
>> "next(int)". This method was indeed letting users assume that it is
>> able to generate _less than 32 bits_ whereas the randomness
>> generators implemented in CM *always* generate exactly 32
>> (hopefully random) bits.
>>
>> I stand guilty on the last count as I do indeed not always "value
>> laziness as a virtue": I indeed attribute more value to design (and
>> code) aesthetics than to laziness. IMO, ugly code is often an early
>> hint that the design is broken, even if the functionality may not
>> be (yet?). [But note again that this change was not "just because
>> of aesthetic reasons"; in the wording of your reference, I think
>> that "just because" is important.]
>>
>> In a real community,[2] you'd value that some people are willing
>> to tackle different tasks than you do. Rather than stifling any
>> change on the sole ground that it is a change, it would be less
>> "draining" on the community if reviewers would only voice concrete
>> concerns about the resulting code, and not just assume that the
>> coder's motivation is pointless.[3]
>>
>> I understand that something can more likely become broken when it
>> is being touched rather than when left alone. Of course, I do! But
>> with the help of an extensive and sensitive test suite, I felt I
>> could give this small refactoring a try, being fairly confident
>> that mistakes would not go unnoticed. Your doubting that the test
>> suite could let this happen should question our assuming that it
>> could assess the correct behaviour of the previous code.
>> Alternately, all of the numerous tests passing should mean that
>> the new code is not buggier; and visual inspection can assess that
>> it cannot be slower.[4]
>>
>> My last thought about how standard the method "next(int)" is, I let
>> it be conveyed by what is not mentioned in the following
>> unquestionably standard source:
>> http://docs.oracle.com/javase/8/docs/api/java/util/SplittableRandom.html
>>
>>
>> [I could have made additional comments on how various suggestions in
>> http://www.apachecon.com/eu2007/materials/ac2006.2.pdf are either
>> not applied in the CM project, or not taken "with a grain of salt"
>> whenever it suits you. But this post has already drained me far
>> too much.]
>>
>> Gilles
>>
>> [1] I can make mistakes (and I did, as told previously) in
>> "fiddling" with the code, but that can be spotted relatively easily
>> by inspecting three one-line changes commits and their few lines
>> consequences on the generic methods where "nextInt()" replaced
>> "next(int)", mutatis mutandis, in a single and quite small class.
>> That is a far cry from "researching the algorithms". [2] To be
>> contrasted with the "common good" for which your stand against
>> changes may be the right one for many (conservative) CM users. [3]
>> I can assure you that it is quite draining to have to defend any
>> and every change, especially when they are obviously towards
>> greater "standardness", even when they would occur in a new major
>> version, against an opposition that is only based on a conservatism
>> argument: When the code can be made more standard or more elegant
>> or more flexible or more state-of-the-art, it is deemed not
>> necessary because "we've always done it that way". This is a
>> general remark about other discussions. Here I totally agree that I
>> favoured non-standard Java in trashing "next(int bits)". In other
>> languages, that signature is _not_ "standard". [4] In order to
>> defend a small and technically innocuous modification, I have to
>> myself do some "researching". Mind you, it's "just" reading from
>> web pages edited by people who AFAICT seem to know what they are
>> talking about (but I may be missing something, again). Among other
>> things which I've shared in this post, it appears that a benchmark
>> including the WELL (1204a) RNG indicates that it is 5 to 10 times
>> slower than alternative RNGs. In this light, if you are so worried
>> about performance issues induced by a functionally no-op change,
>> then you probably should not use CM for RNG.
>>
>>>>> 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.
>>>
>>> Have a look at the source code for the JDK generators, for
>>> example.
>>>> 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?).
>>>
>>> Quite possibly, yes, you are 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).
>>> This change is
>>>>
>>>> Gilles
>>>>
>>>>
>>>>> Phil
>>>>>
>>>>>> On 12/28/15 10:23 AM, erans@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).
>>>>>>
>>>>>>
>>>>>> [...]
>>> [1] http://www.apachecon.com/eu2007/materials/ac2006.2.pdf
>>
>>
>> ---------------------------------------------------------------------
>>
>>
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
> ---------------------------------------------------------------------
>
>
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[All] Don't touch that code! (Was: [Math] About the refactoring of
RNGs)
Posted by Gilles <gi...@harfang.homelinux.org>.
Hi.
[Preamble: In the following, I do not ask for opinions about the
design of Commons Math; I report an example of a dysfunctional
"community".]
A few days ago, rather than "shut down for holidays", I took upon me to
respond to the request of a newcomer to the CM discussions (MATH-1300).
I'm certainly not anywhere close to being an expert in PRNG.
But it looked easy enough to make that change.
While browsing through the related code in "o.a.c.m.random", I happened
to
stumble upon one line of code that looked strange (because its sole
purpose
seemed to clear bits that, anyways, would not be used by the caller).
Then there were the usual suspects when doing such an exercise in CM:
duplicate code ("BitsStreamRandomGenerator" vs
"AbstractRandomGenerator"),
and unsafe code, that prompted me to raise questions on this ML, and to
open issues MATH-1307, MATH-1308 and MATH-1309 (itself related to the
old
MATH-734).
My argumentation was crystal-clear.
No technical objection was raised, and I committed the proposed changes
(as
they were shown in an attachment to the corresponding JIRA page).
The commit elicited a flood of negative comments that made me feel I
must
have made some horrendous, and obviously stupid, mistake.
Around the same time I was making those little changes (creating 1 new
file,
deleting 2, and less than 20 lines changed in total) to the development
branch ("master") containing code not supposed to be
backwards-compatible
and not foreseen to be released any time soon, Luc was adding or
modifying
dozens of files (largely more than 1000 lines to supposedly review) to
the
MATH_3_X branch, thus to be included in the 3.6 version of CM to be
imminently released.
There is thus a double-standard reviewing process.[1]
As it turned out, nobody has, at this time, something technically sound
to
argue against the change which I had made.
Instead, there was a barrage of protests that amounted to creating FUD:
FUD 1: "Quite possibly, yes, you are missing something."
Truth: Strong, but baseless, affirmation that I'm wrong in even
thinking
that some code statement could be useless.
FUD 2: "significant refactoring"
Truth: Less than 20 lines were modified (and for many, it's the same
type
of change).
[Moreover, the behaviour and usage of the concrete classes are
unchanged.]
FUD 3: "[...] rebasing to eliminate next(bits) adds nothing."
Truth: New code exposes what the underlying algorithms actually do
while
the previous code indirectly advertised non-existing
functionality.
[The change actually proves that the argument of "next(int)" is
useless (i.e. the implementations always generate more bits than
the
argument would have one to believe).]
FUD 4: "[refactoring] to eliminate the (standard) next(int)"
Truth: Unchecked and ambiguous use of the word "standard".
As far I could see by browsing some source codes in C,
"next(void)"
could be the standard in RNG codes, not "next(int bits)".
The absence of "next(int)" in the newer JDK class
"SplittableRandom"
confirms the impression that "next(int)" could have been,
indeed,
useless for all practical purposes.
FUD 5: Potential breakage
Truth: 544 unit tests show no sign of regression for any of the RNG
implementations.
FUD 6: "The tests cover only nextInt"
Truth: The "RandomGeneratorAbstractTest" class contains unit tests for
*all*
the generic methods. In particular, the tests ensure that the
distribution of the generated "int", "long", "float" and
"double" is
uniform.
FUD 7: "[...] implementation changes to hardened code"
Truth: Undefined use of the word "hardened": A specific unit test
verifies
that some hard-coded sequences are reproduced by the code. It
passes
with the new code, as it did with the old.
[Other than that, the code comes with no warranty (applies to
*both*
the current version and the previous one).]
FUD 8: "[...] invariants may be broken by these changes"
Truth: Visual inspection of 1 line of code per generic method and 1
line of
code per RNG can prove whether there is a mistake or not.
FUD 9: "[...] possibility of introducing [...] performance issues."
Truth: Visual inspection of 1 line of code per generic method and 1
line of
code per RNG shows a reduction of the number of basic operations
(one
shift and one subtraction in each concrete RNG implementation).
Moreover, IIUC the following results of a micro-benchmark
comparison,
no significant change in performance can be detected.
[Although the change is for the better in almost all cases.]
+----------+
| MATH_3_X |
+----------+
nextInt() (calls per timed block: 2000000, timed blocks: 100, time
unit: ms)
name time/call std error total time ratio
difference
JDKRandomGenerator 1.07711988e-05 3.96937236e-06 2.1542e+03 1.0000e+00
0.00000000e+00
MersenneTwister 1.10205274e-05 2.22826722e-06 2.2041e+03 1.0231e+00
4.98657110e+01
Well512a 1.33509150e-05 1.81097101e-06 2.6702e+03 1.2395e+00
5.15943241e+02
Well1024a 1.30304620e-05 9.68410328e-07 2.6061e+03 1.2098e+00
4.51852641e+02
Well19937a 1.56130116e-05 2.14122318e-06 3.1226e+03 1.4495e+00
9.68362551e+02
Well19937c 1.47900777e-05 1.21972362e-06 2.9580e+03 1.3731e+00
8.03775773e+02
Well44497a 1.73521386e-05 8.06996030e-07 3.4704e+03 1.6110e+00
1.31618795e+03
Well44497b 1.76055476e-05 5.14973607e-07 3.5211e+03 1.6345e+00
1.36686975e+03
ISAAC 1.20243626e-05 7.88169183e-07 2.4049e+03 1.1163e+00
2.50632757e+02
nextDouble() (calls per timed block: 2000000, timed blocks: 100, time
unit: ms)
name time/call std error total time ratio
difference
JDKRandomGenerator 2.21792835e-05 2.41114910e-06 4.4359e+03 1.0000e+00
0.00000000e+00
MersenneTwister 1.97174235e-05 2.03446115e-06 3.9435e+03 8.8900e-01
-4.92371995e+02
Well512a 2.12618083e-05 1.88123228e-06 4.2524e+03 9.5863e-01
-1.83495042e+02
Well1024a 2.41596883e-05 2.57029116e-06 4.8319e+03 1.0893e+00
3.96080966e+02
Well19937a 2.66921255e-05 1.49574930e-06 5.3384e+03 1.2035e+00
9.02568405e+02
Well19937c 2.92679702e-05 1.33918527e-06 5.8536e+03 1.3196e+00
1.41773734e+03
Well44497a 3.36010178e-05 2.73487416e-06 6.7202e+03 1.5150e+00
2.28434687e+03
Well44497b 3.59767542e-05 1.08315236e-06 7.1954e+03 1.6221e+00
2.75949415e+03
ISAAC 2.21894735e-05 1.53691118e-06 4.4379e+03 1.0005e+00
2.03801200e+00
nextLong() (calls per timed block: 2000000, timed blocks: 100, time
unit: ms)
name time/call std error total time ratio
difference
JDKRandomGenerator 2.34176352e-05 2.20143373e-06 4.6835e+03 1.0000e+00
0.00000000e+00
MersenneTwister 1.94922075e-05 1.69613941e-06 3.8984e+03 8.3237e-01
-7.85085547e+02
Well512a 2.49888171e-05 1.35979076e-06 4.9978e+03 1.0671e+00
3.14236383e+02
Well1024a 2.33254629e-05 9.16091045e-07 4.6651e+03 9.9606e-01
-1.84344700e+01
Well19937a 2.61210825e-05 7.35362338e-07 5.2242e+03 1.1154e+00
5.40689466e+02
Well19937c 2.77540845e-05 2.52800203e-06 5.5508e+03 1.1852e+00
8.67289861e+02
Well44497a 3.29713155e-05 2.55438277e-06 6.5943e+03 1.4080e+00
1.91073605e+03
Well44497b 3.42167709e-05 1.27562024e-06 6.8434e+03 1.4612e+00
2.15982714e+03
ISAAC 2.15235364e-05 1.57741593e-06 4.3047e+03 9.1912e-01
-3.78819767e+02
+--------+
| master |
+--------+
nextInt() (calls per timed block: 2000000, timed blocks: 100, time
unit: ms)
name time/call std error total time ratio
difference
JDKRandomGenerator 1.34103050e-05 2.45357474e-06 2.6821e+03 1.0000e+00
0.00000000e+00
MersenneTwister 1.14748419e-05 9.90008146e-07 2.2950e+03 8.5567e-01
-3.87092621e+02
Well512a 1.34941515e-05 1.69129854e-06 2.6988e+03 1.0063e+00
1.67693020e+01
Well1024a 1.50723034e-05 8.03930622e-07 3.0145e+03 1.1239e+00
3.32399667e+02
Well19937a 1.38263157e-05 5.69889705e-06 2.7653e+03 1.0310e+00
8.32021280e+01
Well19937c 1.59393823e-05 2.35562440e-06 3.1879e+03 1.1886e+00
5.05815457e+02
Well44497a 1.93589129e-05 2.64356495e-06 3.8718e+03 1.4436e+00
1.18972157e+03
Well44497b 2.03862615e-05 2.14390340e-06 4.0773e+03 1.5202e+00
1.39519129e+03
ISAAC 1.32823857e-05 2.02074578e-06 2.6565e+03 9.9046e-01
-2.55838710e+01
nextDouble() (calls per timed block: 2000000, timed blocks: 100, time
unit: ms)
name time/call std error total time ratio
difference
JDKRandomGenerator 2.32352997e-05 1.76107593e-06 4.6471e+03 1.0000e+00
0.00000000e+00
MersenneTwister 1.95680107e-05 5.15377258e-07 3.9136e+03 8.4217e-01
-7.33457809e+02
Well512a 2.40500380e-05 2.50013851e-06 4.8100e+03 1.0351e+00
1.62947648e+02
Well1024a 2.48173367e-05 2.93429135e-06 4.9635e+03 1.0681e+00
3.16407385e+02
Well19937a 2.75334969e-05 2.45923272e-06 5.5067e+03 1.1850e+00
8.59639442e+02
Well19937c 2.84004673e-05 1.95285979e-06 5.6801e+03 1.2223e+00
1.03303351e+03
Well44497a 3.55292049e-05 2.88860968e-06 7.1058e+03 1.5291e+00
2.45878103e+03
Well44497b 3.69400758e-05 1.55845565e-06 7.3880e+03 1.5898e+00
2.74095521e+03
ISAAC 2.21769477e-05 2.20944591e-06 4.4354e+03 9.5445e-01
-2.11670411e+02
nextLong() (calls per timed block: 2000000, timed blocks: 100, time
unit: ms)
name time/call std error total time ratio
difference
JDKRandomGenerator 2.24688238e-05 1.33151962e-06 4.4938e+03 1.0000e+00
0.00000000e+00
MersenneTwister 1.81175642e-05 1.63071216e-06 3.6235e+03 8.0634e-01
-8.70251910e+02
Well512a 2.27232582e-05 1.06753850e-06 4.5447e+03 1.0113e+00
5.08868800e+01
Well1024a 2.27862427e-05 1.59353328e-06 4.5572e+03 1.0141e+00
6.34837960e+01
Well19937a 2.50605559e-05 1.00839808e-06 5.0121e+03 1.1153e+00
5.18346431e+02
Well19937c 2.68680821e-05 1.11341253e-06 5.3736e+03 1.1958e+00
8.79851660e+02
Well44497a 3.29918582e-05 7.18500728e-07 6.5984e+03 1.4683e+00
2.10460688e+03
Well44497b 3.47592845e-05 7.83038103e-07 6.9519e+03 1.5470e+00
2.45809215e+03
ISAAC 1.99627637e-05 1.40470250e-06 3.9926e+03 8.8847e-01
-5.01212018e+02
In light of the above, I feel I can call foul on a behaviour that is
fully focused
on stymieing (even mildly) "revolutionary"[2] endeavours.
A supportive community would have engaged in a constructive discussion
(if it
felt that bad side-effects could ensue[3]), rather than being
affirmative that some
parts of the library are out-of-bounds (for me).
Regards,
Gilles
[1] I'd rather not enter in the "meaning of meritocracy" debate; I'm
not implying that
I have the same skills as Luc and that I must benefit of the same
trust. Here we
are considering a specific situation where on the one hand massive
amounts of code
are being accepted without a single review, while on the other hand
an easily
reviewable code (reading 20 lines and/or "mvn test") *must* obtain
clearance from
a PRNG expert (who is knowingly absent from these neighbourhoods).
[2] http://www.apachecon.com/eu2007/materials/ac2006.2.pdf (page 186).
[3] How about simply file a JIRA report (like: "Make sure no corners
were cut in commit
xxxx") rather than starting a FUD campaign on this list?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] About the refactoring of RNGs
Posted by Phil Steitz <ph...@gmail.com>.
> On Dec 29, 2015, at 8:41 AM, Gilles <gi...@harfang.homelinux.org> wrote:
>
>> On Mon, 28 Dec 2015 20:33:24 -0700, Phil Steitz wrote:
>>> On 12/28/15 8:08 PM, Gilles wrote:
>>>> 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.
>>
>> No. Not sufficient. What you have done is changed the internal
>> implementation of all of the Bitstream generators. I am not
>> convinced that you have not broken anything. I will have to do the
>> testing myself. I see no point in fiddling with the internals of
>> this code that has had a lot of eyeballs and testing on it. I was
>> not personally looking forward to researching the algorithms to make
>> sure any invariants may be broken by these changes; but I am now
>> going to have to do this. I have to ask why. Please at some point
>> read [1], especially the sections on "Avoid Flexibility Syndrom" and
>> "Value Laziness as a Virtue." Gratuitous refactoring drains
>> community energy.
>
> It seems that again you don't read what I write.[1]
> Hence the above paragraph is spreading
> F -> "I am not convinced that you have not broken anything."
> U -> "I will have to do the testing myself."
> D -> "I see no point in fiddling [...]"
> .
>
> Or maybe I have to rant about email communication.
> Please reread the thread to fully appreciate that you could have shared
> your doubts among the opinions which you gave about some of my questions.
> When the reply answers to only some of the direct questions, the OP is
> legitimately tempted to assume that the non-comment is akin to "I don't
> care" (as in "left to the judgment of the one who does the job").
>
> As is often the case, you can dislike my ideas of improvements (ideas
> that come on the basis of the information provided by what the code
> does) but I don't appreciate the use of the word "gratuitous", given
> that I clearly stated the purpose of making explicit what the code
> actually does (that is, *generating* 32 random bits and not the number
> of bits passed as a parameter to "next(int)").
> So, the code is now self-documenting; it is a small change, to be sure,
> but hardly gratuitous.
I did not respond to the original question about eliminating next(int) because I was not (still am not) expert in the internals of how the generators work and how the generated ints-of-bits are transformed to make doubles, gaussians, etc. I was hoping someone who was expert would chime in and say either "don't do that" or "that's ok". That did not happen before you committed the changes. That obligates us to review them. The tests cover only nextInt so we need to convince ourselves that the transformations (which have all changed) are still correct. That is in my mind just extra work generated for the community. I would rather see our cycles go toward getting 3.6 out.
I stand by my assertion that the rebasing to eliminate next(bits) adds nothing. If you can demonstrate via benchmarks this it is faster, I will retract that statement.
I know we disagree fundamentally on the priorities of [math]. To me, stability, correctness and ease of use (including ease of upgrades) are paramount. That means you don't make implementation changes to hardened code without a good reason and you limit incompatible change to what is necessary to deliver practical benefit to users in new features or bug fixes. I was +1 to dropping AbstractRandomGenerator and just leaving the bitstream generators in place. That is a simplifying change.
>
> I actually did go some way towards "Avoiding [a] Flexibility Syndrom"
> that *was* present in a seeming flexibility of "next(int)".
> This method was indeed letting users assume that it is able to generate
> _less than 32 bits_ whereas the randomness generators implemented in CM
> *always* generate exactly 32 (hopefully random) bits.
>
> I stand guilty on the last count as I do indeed not always "value
> laziness as a virtue": I indeed attribute more value to design (and code)
> aesthetics than to laziness.
> IMO, ugly code is often an early hint that the design is broken, even
> if the functionality may not be (yet?).
> [But note again that this change was not "just because of aesthetic
> reasons"; in the wording of your reference, I think that "just because"
> is important.]
>
> In a real community,[2] you'd value that some people are willing to
> tackle different tasks than you do.
> Rather than stifling any change on the sole ground that it is a change,
> it would be less "draining" on the community if reviewers would only
> voice concrete concerns about the resulting code, and not just assume
> that the coder's motivation is pointless.[3]
>
> I understand that something can more likely become broken when it is
> being touched rather than when left alone. Of course, I do!
> But with the help of an extensive and sensitive test suite, I felt I
> could give this small refactoring a try, being fairly confident that
> mistakes would not go unnoticed.
> Your doubting that the test suite could let this happen should question
> our assuming that it could assess the correct behaviour of the previous
> code. Alternately, all of the numerous tests passing should mean that the
> new code is not buggier; and visual inspection can assess that it cannot
> be slower.[4]
>
> My last thought about how standard the method "next(int)" is, I let it
> be conveyed by what is not mentioned in the following unquestionably
> standard source:
> http://docs.oracle.com/javase/8/docs/api/java/util/SplittableRandom.html
>
> [I could have made additional comments on how various suggestions in
> http://www.apachecon.com/eu2007/materials/ac2006.2.pdf
> are either not applied in the CM project, or not taken "with a grain of
> salt" whenever it suits you. But this post has already drained me far
> too much.]
>
> Gilles
>
> [1] I can make mistakes (and I did, as told previously) in "fiddling" with
> the code, but that can be spotted relatively easily by inspecting three
> one-line changes commits and their few lines consequences on the generic
> methods where "nextInt()" replaced "next(int)", mutatis mutandis, in a
> single and quite small class.
> That is a far cry from "researching the algorithms".
> [2] To be contrasted with the "common good" for which your stand against
> changes may be the right one for many (conservative) CM users.
> [3] I can assure you that it is quite draining to have to defend any and
> every change, especially when they are obviously towards greater
> "standardness", even when they would occur in a new major version,
> against an opposition that is only based on a conservatism argument:
> When the code can be made more standard or more elegant or more flexible
> or more state-of-the-art, it is deemed not necessary because "we've always
> done it that way".
> This is a general remark about other discussions. Here I totally agree
> that I favoured non-standard Java in trashing "next(int bits)". In other
> languages, that signature is _not_ "standard".
> [4] In order to defend a small and technically innocuous modification, I have
> to myself do some "researching". Mind you, it's "just" reading from web
> pages edited by people who AFAICT seem to know what they are talking about
> (but I may be missing something, again).
> Among other things which I've shared in this post, it appears that a
> benchmark including the WELL (1204a) RNG indicates that it is 5 to 10 times
> slower than alternative RNGs.
> In this light, if you are so worried about performance issues induced by a
> functionally no-op change, then you probably should not use CM for RNG.
>
>>>> 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.
>>
>> Have a look at the source code for the JDK generators, for example.
>>> 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?).
>>
>> Quite possibly, yes, you are 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).
>> This change is
>>>
>>> Gilles
>>>
>>>
>>>> Phil
>>>>
>>>>> On 12/28/15 10:23 AM, erans@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).
>>>>>
>>>>>
>>>>> [...]
>> [1] http://www.apachecon.com/eu2007/materials/ac2006.2.pdf
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] About the refactoring of RNGs
Posted by Gilles <gi...@harfang.homelinux.org>.
On Mon, 28 Dec 2015 20:33:24 -0700, Phil Steitz wrote:
> On 12/28/15 8:08 PM, Gilles wrote:
>> 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.
>
> No. Not sufficient. What you have done is changed the internal
> implementation of all of the Bitstream generators. I am not
> convinced that you have not broken anything. I will have to do the
> testing myself. I see no point in fiddling with the internals of
> this code that has had a lot of eyeballs and testing on it. I was
> not personally looking forward to researching the algorithms to make
> sure any invariants may be broken by these changes; but I am now
> going to have to do this. I have to ask why. Please at some point
> read [1], especially the sections on "Avoid Flexibility Syndrom" and
> "Value Laziness as a Virtue." Gratuitous refactoring drains
> community energy.
It seems that again you don't read what I write.[1]
Hence the above paragraph is spreading
F -> "I am not convinced that you have not broken anything."
U -> "I will have to do the testing myself."
D -> "I see no point in fiddling [...]"
.
Or maybe I have to rant about email communication.
Please reread the thread to fully appreciate that you could have shared
your doubts among the opinions which you gave about some of my
questions.
When the reply answers to only some of the direct questions, the OP is
legitimately tempted to assume that the non-comment is akin to "I don't
care" (as in "left to the judgment of the one who does the job").
As is often the case, you can dislike my ideas of improvements (ideas
that come on the basis of the information provided by what the code
does) but I don't appreciate the use of the word "gratuitous", given
that I clearly stated the purpose of making explicit what the code
actually does (that is, *generating* 32 random bits and not the number
of bits passed as a parameter to "next(int)").
So, the code is now self-documenting; it is a small change, to be sure,
but hardly gratuitous.
I actually did go some way towards "Avoiding [a] Flexibility Syndrom"
that *was* present in a seeming flexibility of "next(int)".
This method was indeed letting users assume that it is able to generate
_less than 32 bits_ whereas the randomness generators implemented in CM
*always* generate exactly 32 (hopefully random) bits.
I stand guilty on the last count as I do indeed not always "value
laziness as a virtue": I indeed attribute more value to design (and
code)
aesthetics than to laziness.
IMO, ugly code is often an early hint that the design is broken, even
if the functionality may not be (yet?).
[But note again that this change was not "just because of aesthetic
reasons"; in the wording of your reference, I think that "just because"
is important.]
In a real community,[2] you'd value that some people are willing to
tackle different tasks than you do.
Rather than stifling any change on the sole ground that it is a change,
it would be less "draining" on the community if reviewers would only
voice concrete concerns about the resulting code, and not just assume
that the coder's motivation is pointless.[3]
I understand that something can more likely become broken when it is
being touched rather than when left alone. Of course, I do!
But with the help of an extensive and sensitive test suite, I felt I
could give this small refactoring a try, being fairly confident that
mistakes would not go unnoticed.
Your doubting that the test suite could let this happen should question
our assuming that it could assess the correct behaviour of the previous
code. Alternately, all of the numerous tests passing should mean that
the
new code is not buggier; and visual inspection can assess that it
cannot
be slower.[4]
My last thought about how standard the method "next(int)" is, I let it
be conveyed by what is not mentioned in the following unquestionably
standard source:
http://docs.oracle.com/javase/8/docs/api/java/util/SplittableRandom.html
[I could have made additional comments on how various suggestions in
http://www.apachecon.com/eu2007/materials/ac2006.2.pdf
are either not applied in the CM project, or not taken "with a grain of
salt" whenever it suits you. But this post has already drained me far
too much.]
Gilles
[1] I can make mistakes (and I did, as told previously) in "fiddling"
with
the code, but that can be spotted relatively easily by inspecting
three
one-line changes commits and their few lines consequences on the
generic
methods where "nextInt()" replaced "next(int)", mutatis mutandis,
in a
single and quite small class.
That is a far cry from "researching the algorithms".
[2] To be contrasted with the "common good" for which your stand
against
changes may be the right one for many (conservative) CM users.
[3] I can assure you that it is quite draining to have to defend any
and
every change, especially when they are obviously towards greater
"standardness", even when they would occur in a new major version,
against an opposition that is only based on a conservatism
argument:
When the code can be made more standard or more elegant or more
flexible
or more state-of-the-art, it is deemed not necessary because "we've
always
done it that way".
This is a general remark about other discussions. Here I totally
agree
that I favoured non-standard Java in trashing "next(int bits)". In
other
languages, that signature is _not_ "standard".
[4] In order to defend a small and technically innocuous modification,
I have
to myself do some "researching". Mind you, it's "just" reading from
web
pages edited by people who AFAICT seem to know what they are
talking about
(but I may be missing something, again).
Among other things which I've shared in this post, it appears that
a
benchmark including the WELL (1204a) RNG indicates that it is 5 to
10 times
slower than alternative RNGs.
In this light, if you are so worried about performance issues
induced by a
functionally no-op change, then you probably should not use CM for
RNG.
>>> 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.
>
> Have a look at the source code for the JDK generators, for example.
>> 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?).
>
> Quite possibly, yes, you are 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).
>>
> This change is
>>
>> Gilles
>>
>>
>>> Phil
>>>
>>> On 12/28/15 10:23 AM, erans@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).
>>>>
>>>>
>>>> [...]
>>
> [1] http://www.apachecon.com/eu2007/materials/ac2006.2.pdf
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] About the refactoring of RNGs (Was: [01/18] [math]
MATH-1307)
Posted by Phil Steitz <ph...@gmail.com>.
On 12/29/15 2:33 AM, Luc Maisonobe wrote:
> Hi all,
>
> Le 29/12/2015 09:21, Thomas Neidhart a écrit :
>> On 12/29/2015 04:33 AM, Phil Steitz wrote:
>>> On 12/28/15 8:08 PM, Gilles wrote:
>>>> 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.
>>> No. Not sufficient. What you have done is changed the internal
>>> implementation of all of the Bitstream generators. I am not
>>> convinced that you have not broken anything. I will have to do the
>>> testing myself. I see no point in fiddling with the internals of
>>> this code that has had a lot of eyeballs and testing on it. I was
>>> not personally looking forward to researching the algorithms to make
>>> sure any invariants may be broken by these changes; but I am now
>>> going to have to do this. I have to ask why. Please at some point
>>> read [1], especially the sections on "Avoid Flexibility Syndrom" and
>>> "Value Laziness as a Virtue." Gratuitous refactoring drains
>>> community energy.
>> +1, on top of that I think we should aim to refactor the parts that
>> really need refactoring and try to keep the number of incompatibilities
>> to the 3_X branch as minimal as possible.
>>
>> Thomas
>>
>>>>> 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.
> In all the papers I have read concerning pseudo random number
> generation, the basic model was based on small chunks of bits,
> much of the time the size of an int because this is what computer
> manages directly (they have no provision to manage chunks of 5 or
> 11 bits for example).
That's what I thought. Internally, is it correct to assume that the
generation is always in int-sized blocks so Gilles is correct that
the bits parameter is only used for output truncation?
>
> Deriving other primitive types from this (boolean, long, double) is
> really an add-on. I even asked an expert about the (Pierre L'Ecuyer
> if I remember well) about some explanations for converting to double
> (which is simply done by multiplying by a constant representing the
> weight of the least significant bit in order to constrain the range to
> [0; 1]). His answer was that this ensured the theoretical mathematical
> proofs that apply to uniform distribution still apply, as only this
> case (uniformity over a multi-dimensional integral grid) has been
> studied. It seems nothing has been studied about using the exponential
> features of floating point representation in relationship with
> double random number generation directly.
Makes sense. The - excellent - tests included with the Well,
Mersenne and Isaac generators verify (as Gilles states) that
nextInt() itself is likely unaffected by the changes above. What I
am worried about is the conversions. Do those changes look correct
to you? That is what needs to be carefully reviewed and tested.
>
> Hence everybody starts from int, and the mathematicians proved us
> this method works and some properties are preserved (multi-dimensional
> independance, long period, ...) that are essential typically for
> Monte-Carlo analyses.
>
> I know nothing about random number generation for secure application
> like cryptograpgy, except that it requires completely different
> properties, often completely opposite to what is needed for
> Monte-Carlo analysis. As an example, it should be impossible to
> reproduce a secure sequence (it cannot be deterministic), whereas in
> Monte-Carlo we *want* it to be reproducible if we reuse the same seed.
>
>>> Have a look at the source code for the JDK generators, for example.
>>>> 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?).
>>> Quite possibly, yes, you are missing something.
> I would guess it is linked to performance consideration. Pseudo
> random number generation is sometimes put under very heavy stress
> with billions of numbers generated. It should run extremelly fast,
> and the algorithms have been designed to have tremendously long periods
> (things like 2^19937 -1). With such long periods, we can waste 31
> bits each time we produce 1 bit if it saves some overhead.
>
> best regards,
> Luc
>
>>>> 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).
>>>>
>>> This change is
>>>> Gilles
>>>>
>>>>
>>>>> Phil
>>>>>
>>>>> On 12/28/15 10:23 AM, erans@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).
>>>>>>
>>>>>>
>>>>>> [...]
>>> [1] http://www.apachecon.com/eu2007/materials/ac2006.2.pdf
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] About the refactoring of RNGs
Posted by Thomas Neidhart <th...@gmail.com>.
On 12/29/2015 05:10 PM, Gilles wrote:
> On Tue, 29 Dec 2015 10:33:15 +0100, Luc Maisonobe wrote:
>> Hi all,
>>
>> Le 29/12/2015 09:21, Thomas Neidhart a écrit :
>>> On 12/29/2015 04:33 AM, Phil Steitz wrote:
>>>> On 12/28/15 8:08 PM, Gilles wrote:
>>>>> 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.
>>>>
>>>> No. Not sufficient. What you have done is changed the internal
>>>> implementation of all of the Bitstream generators. I am not
>>>> convinced that you have not broken anything. I will have to do the
>>>> testing myself. I see no point in fiddling with the internals of
>>>> this code that has had a lot of eyeballs and testing on it. I was
>>>> not personally looking forward to researching the algorithms to make
>>>> sure any invariants may be broken by these changes; but I am now
>>>> going to have to do this. I have to ask why. Please at some point
>>>> read [1], especially the sections on "Avoid Flexibility Syndrom" and
>>>> "Value Laziness as a Virtue." Gratuitous refactoring drains
>>>> community energy.
>>>
>>> +1, on top of that I think we should aim to refactor the parts that
>>> really need refactoring
>
> Though I could have liked to say as much on the parts of the library
> were my changes were much criticized because they failed to produce
> a perfect design (which the previous one wasn't either), I would have
> refrained to tell volunteers what they should do or not.
when I have seen the commit I was already thinking of writing a comment
about it but refrained until Phil was doing so. Afaict, your refactoring
moved the bit-shifting logic from one method (next(int)) to others
(nextDouble(), ...) in a slightly different way. On top of that, some
classes have been removed and added and I do not clearly see how this
has improved something, but maybe it was a change in the right direction
as Luc pointed out.
Nobody blames anybody for something, I believe, instead I have the
impression that people are focused on good technical solutions and this
sometimes means that you get criticism (and I got this as well a lot,
which can be quite frustrating).
I just got the impression lately that some people want to completely
refactor CM for the 4.0 release and I wonder if this will do any good to
the project, especially in areas where there is really no need to
further refactor anything.
>>> and try to keep the number of incompatibilities
>>> to the 3_X branch as minimal as possible.
>
> I clearly and not surprisingly do not subscribe to that goal.
> And the recent discussions about RERO and "experimental" releases
> certainly were getting to a completely different consensus.
I am not sure what this has to do with my comment. RERO just means that
we should release more often, but you can only do this if you are not
introducing incompatibilities.
The discussion about "experimental" releases was just brain-storming
imho. There was no decision or real consensus on how to achieve this,
just some ideas that are at best very non-standard and in some cases
impractical.
I fear that Apache Commons is not the right place for doing such things.
btw. I proposed to use the scheme from guava to explicitly mark new
features with a Beta annotation to allow possible incompatible changes
later on, which I find much more practical, especially considering the
limited man-power and release cycles.
Thomas
>>>>>> 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.
>>
>> In all the papers I have read concerning pseudo random number
>> generation, the basic model was based on small chunks of bits,
>> much of the time the size of an int because this is what computer
>> manages directly (they have no provision to manage chunks of 5 or
>> 11 bits for example).
>>
>> Deriving other primitive types from this (boolean, long, double) is
>> really an add-on. I even asked an expert about the (Pierre L'Ecuyer
>> if I remember well) about some explanations for converting to double
>> (which is simply done by multiplying by a constant representing the
>> weight of the least significant bit in order to constrain the range to
>> [0; 1]). His answer was that this ensured the theoretical mathematical
>> proofs that apply to uniform distribution still apply, as only this
>> case (uniformity over a multi-dimensional integral grid) has been
>> studied. It seems nothing has been studied about using the exponential
>> features of floating point representation in relationship with
>> double random number generation directly.
>>
>> Hence everybody starts from int,
>
> [Or a "long", as I could observe in some other source codes.]
>
> Hence, do you agree that my move to "nextInt()" was a sensible one?
>
>> and the mathematicians proved us
>> this method works and some properties are preserved (multi-dimensional
>> independance, long period, ...) that are essential typically for
>> Monte-Carlo analyses.
>>
>> I know nothing about random number generation for secure application
>> like cryptograpgy, except that it requires completely different
>> properties, often completely opposite to what is needed for
>> Monte-Carlo analysis. As an example, it should be impossible to
>> reproduce a secure sequence (it cannot be deterministic), whereas in
>> Monte-Carlo we *want* it to be reproducible if we reuse the same seed.
>>
>>>>
>>>> Have a look at the source code for the JDK generators, for example.
>>>>> 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?).
>>>>
>>>> Quite possibly, yes, you are missing something.
>>
>> I would guess it is linked to performance consideration. Pseudo
>> random number generation is sometimes put under very heavy stress
>> with billions of numbers generated. It should run extremelly fast,
>> and the algorithms have been designed to have tremendously long periods
>> (things like 2^19937 -1). With such long periods, we can waste 31
>> bits each time we produce 1 bit if it saves some overhead.
>
> Hence I did not misunderstand that bits are wasted (and I guess will
> always be if performance is dependent on the CPU architecture).
>
> IIUC the CM code, discarding those bits was unnecessary since the
> discarded ones were never used (and I guess could never be, as they
> are replaced by a non-random sequence of zeroes...).
> Hence I suppose that a bug could exist if the caller assumes that the
> most significant bits of the returned "int" were all zero.
> But nothing of the like can be inferred from the documentation of the
> "standard" method:
>
> http://docs.oracle.com/javase/7/docs/api/java/util/Random.html#next%28int%29
>
>
>
> Thanks, Luc for constructive comments,
> Gilles
>
>>
>> best regards,
>> Luc
>>
>>>>>
>>>>> 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).
>>>>>
>>>> This change is
>>>>>
>>>>> Gilles
>>>>>
>>>>>
>>>>>> Phil
>>>>>>
>>>>>> On 12/28/15 10:23 AM, erans@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).
>>>>>>>
>>>>>>>
>>>>>>> [...]
>>>>>
>>>> [1] http://www.apachecon.com/eu2007/materials/ac2006.2.pdf
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] About the refactoring of RNGs
Posted by Gilles <gi...@harfang.homelinux.org>.
On Tue, 29 Dec 2015 10:33:15 +0100, Luc Maisonobe wrote:
> Hi all,
>
> Le 29/12/2015 09:21, Thomas Neidhart a écrit :
>> On 12/29/2015 04:33 AM, Phil Steitz wrote:
>>> On 12/28/15 8:08 PM, Gilles wrote:
>>>> 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.
>>>
>>> No. Not sufficient. What you have done is changed the internal
>>> implementation of all of the Bitstream generators. I am not
>>> convinced that you have not broken anything. I will have to do the
>>> testing myself. I see no point in fiddling with the internals of
>>> this code that has had a lot of eyeballs and testing on it. I was
>>> not personally looking forward to researching the algorithms to
>>> make
>>> sure any invariants may be broken by these changes; but I am now
>>> going to have to do this. I have to ask why. Please at some point
>>> read [1], especially the sections on "Avoid Flexibility Syndrom"
>>> and
>>> "Value Laziness as a Virtue." Gratuitous refactoring drains
>>> community energy.
>>
>> +1, on top of that I think we should aim to refactor the parts that
>> really need refactoring
Though I could have liked to say as much on the parts of the library
were my changes were much criticized because they failed to produce
a perfect design (which the previous one wasn't either), I would have
refrained to tell volunteers what they should do or not.
>> and try to keep the number of incompatibilities
>> to the 3_X branch as minimal as possible.
I clearly and not surprisingly do not subscribe to that goal.
And the recent discussions about RERO and "experimental" releases
certainly were getting to a completely different consensus.
>>
>> Thomas
>>
>>>>> 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.
>
> In all the papers I have read concerning pseudo random number
> generation, the basic model was based on small chunks of bits,
> much of the time the size of an int because this is what computer
> manages directly (they have no provision to manage chunks of 5 or
> 11 bits for example).
>
> Deriving other primitive types from this (boolean, long, double) is
> really an add-on. I even asked an expert about the (Pierre L'Ecuyer
> if I remember well) about some explanations for converting to double
> (which is simply done by multiplying by a constant representing the
> weight of the least significant bit in order to constrain the range
> to
> [0; 1]). His answer was that this ensured the theoretical
> mathematical
> proofs that apply to uniform distribution still apply, as only this
> case (uniformity over a multi-dimensional integral grid) has been
> studied. It seems nothing has been studied about using the
> exponential
> features of floating point representation in relationship with
> double random number generation directly.
>
> Hence everybody starts from int,
[Or a "long", as I could observe in some other source codes.]
Hence, do you agree that my move to "nextInt()" was a sensible one?
> and the mathematicians proved us
> this method works and some properties are preserved
> (multi-dimensional
> independance, long period, ...) that are essential typically for
> Monte-Carlo analyses.
>
> I know nothing about random number generation for secure application
> like cryptograpgy, except that it requires completely different
> properties, often completely opposite to what is needed for
> Monte-Carlo analysis. As an example, it should be impossible to
> reproduce a secure sequence (it cannot be deterministic), whereas in
> Monte-Carlo we *want* it to be reproducible if we reuse the same
> seed.
>
>>>
>>> Have a look at the source code for the JDK generators, for example.
>>>> 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?).
>>>
>>> Quite possibly, yes, you are missing something.
>
> I would guess it is linked to performance consideration. Pseudo
> random number generation is sometimes put under very heavy stress
> with billions of numbers generated. It should run extremelly fast,
> and the algorithms have been designed to have tremendously long
> periods
> (things like 2^19937 -1). With such long periods, we can waste 31
> bits each time we produce 1 bit if it saves some overhead.
Hence I did not misunderstand that bits are wasted (and I guess will
always be if performance is dependent on the CPU architecture).
IIUC the CM code, discarding those bits was unnecessary since the
discarded ones were never used (and I guess could never be, as they
are replaced by a non-random sequence of zeroes...).
Hence I suppose that a bug could exist if the caller assumes that the
most significant bits of the returned "int" were all zero.
But nothing of the like can be inferred from the documentation of the
"standard" method:
http://docs.oracle.com/javase/7/docs/api/java/util/Random.html#next%28int%29
Thanks, Luc for constructive comments,
Gilles
>
> best regards,
> Luc
>
>>>>
>>>> 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).
>>>>
>>> This change is
>>>>
>>>> Gilles
>>>>
>>>>
>>>>> Phil
>>>>>
>>>>> On 12/28/15 10:23 AM, erans@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).
>>>>>>
>>>>>>
>>>>>> [...]
>>>>
>>> [1] http://www.apachecon.com/eu2007/materials/ac2006.2.pdf
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] About the refactoring of RNGs (Was: [01/18] [math]
MATH-1307)
Posted by Luc Maisonobe <lu...@spaceroots.org>.
Hi all,
Le 29/12/2015 09:21, Thomas Neidhart a écrit :
> On 12/29/2015 04:33 AM, Phil Steitz wrote:
>> On 12/28/15 8:08 PM, Gilles wrote:
>>> 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.
>>
>> No. Not sufficient. What you have done is changed the internal
>> implementation of all of the Bitstream generators. I am not
>> convinced that you have not broken anything. I will have to do the
>> testing myself. I see no point in fiddling with the internals of
>> this code that has had a lot of eyeballs and testing on it. I was
>> not personally looking forward to researching the algorithms to make
>> sure any invariants may be broken by these changes; but I am now
>> going to have to do this. I have to ask why. Please at some point
>> read [1], especially the sections on "Avoid Flexibility Syndrom" and
>> "Value Laziness as a Virtue." Gratuitous refactoring drains
>> community energy.
>
> +1, on top of that I think we should aim to refactor the parts that
> really need refactoring and try to keep the number of incompatibilities
> to the 3_X branch as minimal as possible.
>
> Thomas
>
>>>> 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.
In all the papers I have read concerning pseudo random number
generation, the basic model was based on small chunks of bits,
much of the time the size of an int because this is what computer
manages directly (they have no provision to manage chunks of 5 or
11 bits for example).
Deriving other primitive types from this (boolean, long, double) is
really an add-on. I even asked an expert about the (Pierre L'Ecuyer
if I remember well) about some explanations for converting to double
(which is simply done by multiplying by a constant representing the
weight of the least significant bit in order to constrain the range to
[0; 1]). His answer was that this ensured the theoretical mathematical
proofs that apply to uniform distribution still apply, as only this
case (uniformity over a multi-dimensional integral grid) has been
studied. It seems nothing has been studied about using the exponential
features of floating point representation in relationship with
double random number generation directly.
Hence everybody starts from int, and the mathematicians proved us
this method works and some properties are preserved (multi-dimensional
independance, long period, ...) that are essential typically for
Monte-Carlo analyses.
I know nothing about random number generation for secure application
like cryptograpgy, except that it requires completely different
properties, often completely opposite to what is needed for
Monte-Carlo analysis. As an example, it should be impossible to
reproduce a secure sequence (it cannot be deterministic), whereas in
Monte-Carlo we *want* it to be reproducible if we reuse the same seed.
>>
>> Have a look at the source code for the JDK generators, for example.
>>> 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?).
>>
>> Quite possibly, yes, you are missing something.
I would guess it is linked to performance consideration. Pseudo
random number generation is sometimes put under very heavy stress
with billions of numbers generated. It should run extremelly fast,
and the algorithms have been designed to have tremendously long periods
(things like 2^19937 -1). With such long periods, we can waste 31
bits each time we produce 1 bit if it saves some overhead.
best regards,
Luc
>>>
>>> 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).
>>>
>> This change is
>>>
>>> Gilles
>>>
>>>
>>>> Phil
>>>>
>>>> On 12/28/15 10:23 AM, erans@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).
>>>>>
>>>>>
>>>>> [...]
>>>
>> [1] http://www.apachecon.com/eu2007/materials/ac2006.2.pdf
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] About the refactoring of RNGs (Was: [01/18] [math]
MATH-1307)
Posted by Thomas Neidhart <th...@gmail.com>.
On 12/29/2015 04:33 AM, Phil Steitz wrote:
> On 12/28/15 8:08 PM, Gilles wrote:
>> 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.
>
> No. Not sufficient. What you have done is changed the internal
> implementation of all of the Bitstream generators. I am not
> convinced that you have not broken anything. I will have to do the
> testing myself. I see no point in fiddling with the internals of
> this code that has had a lot of eyeballs and testing on it. I was
> not personally looking forward to researching the algorithms to make
> sure any invariants may be broken by these changes; but I am now
> going to have to do this. I have to ask why. Please at some point
> read [1], especially the sections on "Avoid Flexibility Syndrom" and
> "Value Laziness as a Virtue." Gratuitous refactoring drains
> community energy.
+1, on top of that I think we should aim to refactor the parts that
really need refactoring and try to keep the number of incompatibilities
to the 3_X branch as minimal as possible.
Thomas
>>> 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.
>
> Have a look at the source code for the JDK generators, for example.
>> 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?).
>
> Quite possibly, yes, you are 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).
>>
> This change is
>>
>> Gilles
>>
>>
>>> Phil
>>>
>>> On 12/28/15 10:23 AM, erans@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).
>>>>
>>>>
>>>> [...]
>>
> [1] http://www.apachecon.com/eu2007/materials/ac2006.2.pdf
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] About the refactoring of RNGs (Was: [01/18] [math]
MATH-1307)
Posted by Phil Steitz <ph...@gmail.com>.
On 12/28/15 8:08 PM, Gilles wrote:
> 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.
No. Not sufficient. What you have done is changed the internal
implementation of all of the Bitstream generators. I am not
convinced that you have not broken anything. I will have to do the
testing myself. I see no point in fiddling with the internals of
this code that has had a lot of eyeballs and testing on it. I was
not personally looking forward to researching the algorithms to make
sure any invariants may be broken by these changes; but I am now
going to have to do this. I have to ask why. Please at some point
read [1], especially the sections on "Avoid Flexibility Syndrom" and
"Value Laziness as a Virtue." Gratuitous refactoring drains
community energy.
>
>> 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.
Have a look at the source code for the JDK generators, for example.
> 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?).
Quite possibly, yes, you are 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).
>
This change is
>
> Gilles
>
>
>> Phil
>>
>> On 12/28/15 10:23 AM, erans@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).
>>>
>>>
>>> [...]
>
[1] http://www.apachecon.com/eu2007/materials/ac2006.2.pdf
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[Math] About the refactoring of RNGs (Was: [01/18] [math] MATH-1307)
Posted by Gilles <gi...@harfang.homelinux.org>.
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, erans@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-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [01/18] [math] MATH-1307
Posted by Phil Steitz <ph...@gmail.com>.
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 and the refactored
code and benchmarks to show there is no loss in performance. It
would also be good to have some additional review of this code by
PRNG experts.
Phil
On 12/28/15 10:23 AM, erans@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).
>
>
> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
> Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/4cbb388b
> Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/4cbb388b
> Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/4cbb388b
>
> Branch: refs/heads/master
> Commit: 4cbb388ba9099be121f81d75000acc3af93bf993
> Parents: 7b62d01
> Author: Gilles <er...@apache.org>
> Authored: Mon Dec 28 16:42:55 2015 +0100
> Committer: Gilles <er...@apache.org>
> Committed: Mon Dec 28 16:42:55 2015 +0100
>
> ----------------------------------------------------------------------
> .../math4/random/BaseRandomGenerator.java | 270 +++++++++++++++++++
> .../math4/random/BaseRandomGeneratorTest.java | 129 +++++++++
> 2 files changed, 399 insertions(+)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/commons-math/blob/4cbb388b/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
> ----------------------------------------------------------------------
> diff --git a/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java b/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
> new file mode 100644
> index 0000000..a78fd38
> --- /dev/null
> +++ b/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
> @@ -0,0 +1,270 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.commons.math4.random;
> +
> +import java.io.Serializable;
> +
> +import org.apache.commons.math4.exception.NotStrictlyPositiveException;
> +import org.apache.commons.math4.exception.OutOfRangeException;
> +import org.apache.commons.math4.util.FastMath;
> +
> +/**
> + * Abstract class implementing the methods of the {@link RandomGenerator}
> + * interface in a generic way on the basis of abstract method {@link nextInt()}
> + * to be defined in subclasses.
> + *
> + * It also provides additional utility methods that are not part of the
> + * {@link RandomGenerator} API.
> + *
> + * @since 4.0
> + */
> +public abstract class BaseRandomGenerator
> + implements RandomGenerator,
> + Serializable {
> + /** Identifier for serialization. */
> + private static final long serialVersionUID = 20151227L;
> + /** Next Gaussian. */
> + private double nextGaussian = Double.NaN;
> +
> + /**
> + * {@inheritDoc}
> + *
> + * Basic building block for all the generic methods defined in this class.
> + * It produces the next random number according to a specific algorithm to
> + * be implemented by a subclass.
> + */
> + @Override
> + public abstract int nextInt();
> +
> + /** {@inheritDoc} */
> + @Override
> + public boolean nextBoolean() {
> + return (nextInt() >>> 31) != 0;
> + }
> +
> + /** {@inheritDoc} */
> + @Override
> + public double nextDouble() {
> + final long high = ((long) (nextInt() >>> 6)) << 26;
> + final int low = nextInt() >>> 6;
> + return (high | low) * 0x1.0p-52d;
> + }
> +
> + /** {@inheritDoc} */
> + @Override
> + public float nextFloat() {
> + return (nextInt() >>> 9) * 0x1.0p-23f;
> + }
> +
> + /** {@inheritDoc} */
> + @Override
> + public double nextGaussian() {
> + final double random;
> + if (Double.isNaN(nextGaussian)) {
> + // Generate a new pair of gaussian numbers.
> + final double x = nextDouble();
> + final double y = nextDouble();
> + final double alpha = 2 * FastMath.PI * x;
> + final double r = FastMath.sqrt(-2 * FastMath.log(y));
> + random = r * FastMath.cos(alpha);
> + nextGaussian = r * FastMath.sin(alpha);
> + } else {
> + // Use the second element of the pair already generated.
> + random = nextGaussian;
> + nextGaussian = Double.NaN;
> + }
> +
> + return random;
> + }
> +
> + /**
> + * {@inheritDoc}
> + *
> + * <p>
> + * This default implementation is copied from Apache Harmony
> + * java.util.Random (r929253).
> + * </p>
> + *
> + * <p>Implementation notes:
> + * <ul>
> + * <li>If n is a power of 2, this method returns
> + * {@code (int) ((n * (long) next(31)) >> 31)}.</li>
> + * <li>If n is not a power of 2, what is returned is {@code next(31) % n}
> + * with {@code next(31)} values rejected (i.e. regenerated) until a
> + * value that is larger than the remainder of {@code Integer.MAX_VALUE / n}
> + * is generated. Rejection of this initial segment is necessary to ensure
> + * a uniform distribution.</li>
> + * </ul>
> + * </p>
> + */
> + @Override
> + public int nextInt(int n) throws IllegalArgumentException {
> + if (n > 0) {
> + if ((n & -n) == n) {
> + return (int) ((n * (long) (nextInt() >>> 1)) >> 31);
> + }
> + int bits;
> + int val;
> + do {
> + bits = (nextInt() >>> 1);
> + val = bits % n;
> + } while (bits - val + (n - 1) < 0);
> + return val;
> + }
> +
> + throw new NotStrictlyPositiveException(n);
> + }
> +
> + /** {@inheritDoc} */
> + @Override
> + public long nextLong() {
> + final long high = ((long) nextInt()) << 32;
> + final long low = nextInt() & 0xffffffffL;
> + return high | low;
> + }
> +
> + /**
> + * Returns a pseudorandom, uniformly distributed {@code long} value
> + * between 0 (inclusive) and the specified value (exclusive), drawn from
> + * this random number generator's sequence.
> + *
> + * @param n the bound on the random number to be returned. Must be
> + * positive.
> + * @return a pseudorandom, uniformly distributed {@code long} value
> + * between 0 (inclusive) and n (exclusive).
> + * @throws IllegalArgumentException if n is not positive.
> + */
> + public long nextLong(long n) {
> + if (n > 0) {
> + long bits;
> + long val;
> + do {
> + bits = ((long) (nextInt() >>> 1)) << 32;
> + bits |= ((long) nextInt()) & 0xffffffffL;
> + val = bits % n;
> + } while (bits - val + (n - 1) < 0);
> + return val;
> + }
> +
> + throw new NotStrictlyPositiveException(n);
> + }
> +
> + /**
> + * Clears the cache used by the default implementation of
> + * {@link #nextGaussian}.
> + */
> + public void clear() {
> + nextGaussian = Double.NaN;
> + }
> +
> + /**
> + * Generates random bytes and places them into a user-supplied array.
> + *
> + * <p>
> + * The array is filled with bytes extracted from random integers generated
> + * using {@link #nextInt()}.
> + * This implies that the number of random bytes generated may be larger than
> + * the length of the byte array.
> + * </p>
> + *
> + * @param bytes Array in which to put the generated bytes. Cannot be {@code null}.
> + */
> + @Override
> + public void nextBytes(byte[] bytes) {
> + nextBytesFill(bytes, 0, bytes.length);
> + }
> +
> + /**
> + * Generates random bytes and places them into a user-supplied array.
> + *
> + * <p>
> + * The array is filled with bytes extracted from random integers generated
> + * using {@link #nextInt()}.
> + * This implies that the number of random bytes generated may be larger than
> + * the length of the byte array.
> + * </p>
> + *
> + * @param bytes Array in which to put the generated bytes. Cannot be {@code null}.
> + * @param start Index at which to start inserting the generated bytes.
> + * @param len Number of bytes to insert.
> + * @throws OutOfRangeException if {@code start < 0} or {@code start >= bytes.length}.
> + * @throws OutOfRangeException if {@code len <= 0} or {@code len > bytes.length - start}.
> + */
> + public void nextBytes(byte[] bytes,
> + int start,
> + int len) {
> + if (start < 0 ||
> + start >= bytes.length) {
> + throw new OutOfRangeException(start, 0, bytes.length);
> + }
> + final int max = bytes.length - start;
> + if (len <= 0 ||
> + len > max) {
> + throw new OutOfRangeException(len, 0, max);
> + }
> +
> + nextBytesFill(bytes, start, len);
> + }
> +
> + /**
> + * Generates random bytes and places them into a user-supplied array.
> + *
> + * <p>
> + * The array is filled with bytes extracted from random integers generated
> + * using {@link #nextInt()}.
> + * This implies that the number of random bytes generated may be larger than
> + * the length of the byte array.
> + * </p>
> + *
> + * @param bytes Array in which to put the generated bytes. Cannot be {@code null}.
> + * @param position Index at which to start inserting the generated bytes.
> + * @param length Number of bytes to insert.
> + */
> + private void nextBytesFill(byte[] bytes,
> + int position,
> + int length) {
> + int index = position; // Index of first insertion.
> +
> + // Index of first insertion plus multiple 4 part of length (i.e. length
> + // with two least significant bits unset).
> + final int indexLoopLimit = index + (length & 0x7ffffffc);
> +
> + // Start filling in the byte array, 4 bytes at a time.
> + while (index < indexLoopLimit) {
> + final int random = nextInt();
> + bytes[index++] = (byte) random;
> + bytes[index++] = (byte) (random >>> 8);
> + bytes[index++] = (byte) (random >>> 16);
> + bytes[index++] = (byte) (random >>> 24);
> + }
> +
> + final int indexLimit = position + length; // Index of last insertion + 1.
> +
> + // Fill in the remaining bytes.
> + if (index < indexLimit) {
> + int random = nextInt();
> + while (true) {
> + bytes[index++] = (byte) random;
> + if (index < indexLimit) {
> + random >>>= 8;
> + } else {
> + break;
> + }
> + }
> + }
> + }
> +}
>
> http://git-wip-us.apache.org/repos/asf/commons-math/blob/4cbb388b/src/test/java/org/apache/commons/math4/random/BaseRandomGeneratorTest.java
> ----------------------------------------------------------------------
> diff --git a/src/test/java/org/apache/commons/math4/random/BaseRandomGeneratorTest.java b/src/test/java/org/apache/commons/math4/random/BaseRandomGeneratorTest.java
> new file mode 100644
> index 0000000..40ab9b5
> --- /dev/null
> +++ b/src/test/java/org/apache/commons/math4/random/BaseRandomGeneratorTest.java
> @@ -0,0 +1,129 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.commons.math4.random;
> +
> +import java.util.Random;
> +
> +import org.apache.commons.math4.exception.OutOfRangeException;
> +
> +import org.junit.Assert;
> +import org.junit.Before;
> +import org.junit.Test;
> +
> +/**
> + * Tests for the generic implementations of the methods defined in the
> + * {@link BaseRandomGenerator} class, using the standard {@link Random}
> + * class as the source of randomness.
> + */
> +public class BaseRandomGeneratorTest extends RandomGeneratorAbstractTest {
> + /** To simplify testing of additional utility methods. */
> + protected BaseRandomGenerator baseRandomGenerator;
> +
> + @Before
> + public void setUp() {
> + baseRandomGenerator = (BaseRandomGenerator) generator;
> + }
> +
> + @Override
> + protected RandomGenerator makeGenerator() {
> + final RandomGenerator generator = new TestGenerator();
> + generator.setSeed(1000);
> + return generator;
> + }
> +
> + @Test(expected=OutOfRangeException.class)
> + public void testNextBytesPrecondition1() {
> + final int len = 3;
> + final byte[] b = new byte[len];
> + baseRandomGenerator.nextBytes(b, -1, 1);
> + }
> +
> + @Test(expected=OutOfRangeException.class)
> + public void testNextBytesPrecondition2() {
> + final int len = 3;
> + final byte[] b = new byte[len];
> + baseRandomGenerator.nextBytes(b, len, 0);
> + }
> +
> + @Test(expected=OutOfRangeException.class)
> + public void testNextBytesPrecondition3() {
> + final int len = 3;
> + final byte[] b = new byte[len];
> + baseRandomGenerator.nextBytes(b, 0, 0);
> + }
> +
> + @Test(expected=OutOfRangeException.class)
> + public void testNextBytesPrecondition4() {
> + final int len = 3;
> + final byte[] b = new byte[len];
> + baseRandomGenerator.nextBytes(b, 0, len + 1);
> + }
> +
> + @Test
> + public void testNextBytesSubArray() {
> + final int size = 123;
> + final int insert = 72;
> + final int len = 37;
> +
> + final byte[] buffer = new byte[size];
> + baseRandomGenerator.nextBytes(buffer, insert, len);
> +
> + final byte[] bufferCopy = buffer.clone();
> + baseRandomGenerator.nextBytes(buffer, insert, len);
> +
> + for (int i = 0; i < insert; i++) {
> + Assert.assertEquals(bufferCopy[i], buffer[i]);
> + }
> + final int maxInsert = insert + len;
> + for (int i = insert; i < maxInsert; i++) {
> + Assert.assertNotEquals(bufferCopy[i], buffer[i]);
> + }
> + for (int i = maxInsert; i < size; i++) {
> + Assert.assertEquals(bufferCopy[i], buffer[i]);
> + }
> + }
> +
> + /**
> + * Test RNG delegating to {@link Random}.
> + */
> + private static class TestGenerator extends BaseRandomGenerator {
> + /** Delegate. */
> + private Random random = new Random();
> +
> + @Override
> + public void setSeed(int seed) {
> + random.setSeed(seed);
> + clear();
> + }
> +
> + @Override
> + public void setSeed(int[] seed) {
> + random.setSeed(seed[0]);
> + }
> +
> + @Override
> + public void setSeed(long seed) {
> + random.setSeed((int) seed);
> + }
> +
> + @Override
> + public int nextInt() {
> + // Delegate.
> + return random.nextInt();
> + }
> + }
> +}
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[08/18] [math] Serialization identifier.
Posted by er...@apache.org.
Serialization identifier.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/e34f50dd
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/e34f50dd
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/e34f50dd
Branch: refs/heads/master
Commit: e34f50dd1083fa8ed9fe25fe701b64bd2ea7efc0
Parents: f9e7200
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 17:02:08 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 17:02:08 2015 +0100
----------------------------------------------------------------------
src/main/java/org/apache/commons/math4/random/AbstractWell.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/e34f50dd/src/main/java/org/apache/commons/math4/random/AbstractWell.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/AbstractWell.java b/src/main/java/org/apache/commons/math4/random/AbstractWell.java
index a389d58..b2d61a3 100644
--- a/src/main/java/org/apache/commons/math4/random/AbstractWell.java
+++ b/src/main/java/org/apache/commons/math4/random/AbstractWell.java
@@ -38,7 +38,7 @@ public abstract class AbstractWell
extends BaseRandomGenerator
implements Serializable {
/** Serializable version identifier. */
- private static final long serialVersionUID = 20150228L;
+ private static final long serialVersionUID = 20151228L;
/** Current index in the bytes pool. */
protected int index;
/** Bytes pool. */
[03/18] [math] Javadoc.
Posted by er...@apache.org.
Javadoc.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/6f4f676c
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/6f4f676c
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/6f4f676c
Branch: refs/heads/master
Commit: 6f4f676c4b730f7639deef9af205b596fa0aab8e
Parents: 2a8061f
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 16:47:15 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 16:47:15 2015 +0100
----------------------------------------------------------------------
.../commons/math4/random/RandomGeneratorAbstractTest.java | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/6f4f676c/src/test/java/org/apache/commons/math4/random/RandomGeneratorAbstractTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/RandomGeneratorAbstractTest.java b/src/test/java/org/apache/commons/math4/random/RandomGeneratorAbstractTest.java
index 26d4dfd..1d8af9f 100644
--- a/src/test/java/org/apache/commons/math4/random/RandomGeneratorAbstractTest.java
+++ b/src/test/java/org/apache/commons/math4/random/RandomGeneratorAbstractTest.java
@@ -43,13 +43,13 @@ import org.junit.Ignore;
*/
public abstract class RandomGeneratorAbstractTest extends RandomDataGeneratorTest {
-
- /** RandomGenerator under test */
+ /** Generator to test. */
protected RandomGenerator generator;
/**
* Override this method in subclasses to provide a concrete generator to test.
- * Return a generator seeded with a fixed seed.
+ *
+ * @return a new generator seeded with a fixed seed.
*/
protected abstract RandomGenerator makeGenerator();
[12/18] [math] MATH-1307
Posted by er...@apache.org.
MATH-1307
Remove obsolete class.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/ca01fdf5
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/ca01fdf5
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/ca01fdf5
Branch: refs/heads/master
Commit: ca01fdf5808dcaf5bdb6e1c09a7de70a0adc0d28
Parents: 4fc5b34
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 17:46:59 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 17:46:59 2015 +0100
----------------------------------------------------------------------
.../math4/random/BitsStreamGenerator.java | 218 -------------------
.../math4/random/BitsStreamGeneratorTest.java | 86 --------
2 files changed, 304 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ca01fdf5/src/main/java/org/apache/commons/math4/random/BitsStreamGenerator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/BitsStreamGenerator.java b/src/main/java/org/apache/commons/math4/random/BitsStreamGenerator.java
deleted file mode 100644
index 4cf6823..0000000
--- a/src/main/java/org/apache/commons/math4/random/BitsStreamGenerator.java
+++ /dev/null
@@ -1,218 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.commons.math4.random;
-
-import java.io.Serializable;
-
-import org.apache.commons.math4.exception.NotStrictlyPositiveException;
-import org.apache.commons.math4.util.FastMath;
-
-/** Base class for random number generators that generates bits streams.
- *
- * @since 2.0
- */
-public abstract class BitsStreamGenerator
- implements RandomGenerator,
- Serializable {
- /** Serializable version identifier */
- private static final long serialVersionUID = 20130104L;
- /** Next gaussian. */
- private double nextGaussian;
-
- /**
- * Creates a new random number generator.
- */
- public BitsStreamGenerator() {
- nextGaussian = Double.NaN;
- }
-
- /** {@inheritDoc} */
- @Override
- public abstract void setSeed(int seed);
-
- /** {@inheritDoc} */
- @Override
- public abstract void setSeed(int[] seed);
-
- /** {@inheritDoc} */
- @Override
- public abstract void setSeed(long seed);
-
- /** Generate next pseudorandom number.
- * <p>This method is the core generation algorithm. It is used by all the
- * public generation methods for the various primitive types {@link
- * #nextBoolean()}, {@link #nextBytes(byte[])}, {@link #nextDouble()},
- * {@link #nextFloat()}, {@link #nextGaussian()}, {@link #nextInt()},
- * {@link #next(int)} and {@link #nextLong()}.</p>
- * @param bits number of random bits to produce
- * @return random bits generated
- */
- protected abstract int next(int bits);
-
- /** {@inheritDoc} */
- @Override
- public boolean nextBoolean() {
- return next(1) != 0;
- }
-
- /** {@inheritDoc} */
- @Override
- public void nextBytes(byte[] bytes) {
- // Multiple 4 part of length (i.e. length with two least significant bits unset).
- final int max = bytes.length & 0x7ffffffc;
-
- int index = 0;
- // Start filling in the byte array, 4 bytes at a time.
- while (index < max) {
- final int random = next(32);
- bytes[index++] = (byte) random;
- bytes[index++] = (byte) (random >>> 8);
- bytes[index++] = (byte) (random >>> 16);
- bytes[index++] = (byte) (random >>> 24);
- }
-
- // Fill in the remaing bytes.
- if (index < bytes.length) {
- int random = next(32);
- while (true) {
- bytes[index++] = (byte) random;
- if (index < bytes.length) {
- random >>>= 8;
- } else {
- break;
- }
- }
- }
- }
-
- /** {@inheritDoc} */
- @Override
- public double nextDouble() {
- final long high = ((long) next(26)) << 26;
- final int low = next(26);
- return (high | low) * 0x1.0p-52d;
- }
-
- /** {@inheritDoc} */
- @Override
- public float nextFloat() {
- return next(23) * 0x1.0p-23f;
- }
-
- /** {@inheritDoc} */
- @Override
- public double nextGaussian() {
-
- final double random;
- if (Double.isNaN(nextGaussian)) {
- // generate a new pair of gaussian numbers
- final double x = nextDouble();
- final double y = nextDouble();
- final double alpha = 2 * FastMath.PI * x;
- final double r = FastMath.sqrt(-2 * FastMath.log(y));
- random = r * FastMath.cos(alpha);
- nextGaussian = r * FastMath.sin(alpha);
- } else {
- // use the second element of the pair already generated
- random = nextGaussian;
- nextGaussian = Double.NaN;
- }
-
- return random;
-
- }
-
- /** {@inheritDoc} */
- @Override
- public int nextInt() {
- return next(32);
- }
-
- /**
- * {@inheritDoc}
- * <p>This default implementation is copied from Apache Harmony
- * java.util.Random (r929253).</p>
- *
- * <p>Implementation notes: <ul>
- * <li>If n is a power of 2, this method returns
- * {@code (int) ((n * (long) next(31)) >> 31)}.</li>
- *
- * <li>If n is not a power of 2, what is returned is {@code next(31) % n}
- * with {@code next(31)} values rejected (i.e. regenerated) until a
- * value that is larger than the remainder of {@code Integer.MAX_VALUE / n}
- * is generated. Rejection of this initial segment is necessary to ensure
- * a uniform distribution.</li></ul></p>
- */
- @Override
- public int nextInt(int n) throws IllegalArgumentException {
- if (n > 0) {
- if ((n & -n) == n) {
- return (int) ((n * (long) next(31)) >> 31);
- }
- int bits;
- int val;
- do {
- bits = next(31);
- val = bits % n;
- } while (bits - val + (n - 1) < 0);
- return val;
- }
- throw new NotStrictlyPositiveException(n);
- }
-
- /** {@inheritDoc} */
- @Override
- public long nextLong() {
- final long high = ((long) next(32)) << 32;
- final long low = (next(32)) & 0xffffffffL;
- return high | low;
- }
-
- /**
- * Returns a pseudorandom, uniformly distributed {@code long} value
- * between 0 (inclusive) and the specified value (exclusive), drawn from
- * this random number generator's sequence.
- *
- * @param n the bound on the random number to be returned. Must be
- * positive.
- * @return a pseudorandom, uniformly distributed {@code long}
- * value between 0 (inclusive) and n (exclusive).
- * @throws IllegalArgumentException if n is not positive.
- */
- public long nextLong(long n) throws IllegalArgumentException {
- if (n > 0) {
- long bits;
- long val;
- do {
- bits = ((long) next(31)) << 32;
- bits |= ((long) next(32)) & 0xffffffffL;
- val = bits % n;
- } while (bits - val + (n - 1) < 0);
- return val;
- }
- throw new NotStrictlyPositiveException(n);
- }
-
- /**
- * Clears the cache used by the default implementation of
- * {@link #nextGaussian}.
- */
- public void clear() {
- nextGaussian = Double.NaN;
- }
-
-}
http://git-wip-us.apache.org/repos/asf/commons-math/blob/ca01fdf5/src/test/java/org/apache/commons/math4/random/BitsStreamGeneratorTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/BitsStreamGeneratorTest.java b/src/test/java/org/apache/commons/math4/random/BitsStreamGeneratorTest.java
deleted file mode 100644
index 7f43ccb..0000000
--- a/src/test/java/org/apache/commons/math4/random/BitsStreamGeneratorTest.java
+++ /dev/null
@@ -1,86 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.commons.math4.random;
-
-import java.util.Random;
-
-import org.apache.commons.math4.random.BitsStreamGenerator;
-import org.apache.commons.math4.random.RandomGenerator;
-
-/**
- * Test cases for the BitStreamGenerator class
- *
- */
-
-public class BitsStreamGeneratorTest extends RandomGeneratorAbstractTest {
-
- public BitsStreamGeneratorTest() {
- super();
- }
-
- @Override
- protected RandomGenerator makeGenerator() {
- RandomGenerator generator = new TestBitStreamGenerator();
- generator.setSeed(1000);
- return generator;
- }
-
- /**
- * Test BitStreamGenerator using a Random as bit source.
- */
- static class TestBitStreamGenerator extends BitsStreamGenerator {
-
- private static final long serialVersionUID = 1L;
- private BitRandom ran = new BitRandom();
-
- @Override
- public void setSeed(int seed) {
- ran.setSeed(seed);
- clear();
- }
-
- @Override
- public void setSeed(int[] seed) {
- ran.setSeed(seed[0]);
- }
-
- @Override
- public void setSeed(long seed) {
- ran.setSeed((int) seed);
-
- }
-
- @Override
- protected int next(int bits) {
- return ran.nextBits(bits);
- }
- }
-
- /**
- * Extend Random to expose next(bits)
- */
- @SuppressWarnings("serial")
- static class BitRandom extends Random {
- public BitRandom() {
- super();
- }
- public int nextBits(int bits) {
- return next(bits);
- }
- }
-
-}
[14/18] [math] Javadoc.
Posted by er...@apache.org.
Javadoc.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/aaabfe8d
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/aaabfe8d
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/aaabfe8d
Branch: refs/heads/master
Commit: aaabfe8d2d8d8c963310cdbd8fd0e81c50006992
Parents: d8e2f89
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 18:05:05 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 18:05:05 2015 +0100
----------------------------------------------------------------------
.../commons/math4/random/RandomGenerator.java | 4 +--
.../commons/math4/random/package-info.java | 26 +++++++++++---------
2 files changed, 17 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/aaabfe8d/src/main/java/org/apache/commons/math4/random/RandomGenerator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/RandomGenerator.java b/src/main/java/org/apache/commons/math4/random/RandomGenerator.java
index 8e0b5b1..b1e7e5e 100644
--- a/src/main/java/org/apache/commons/math4/random/RandomGenerator.java
+++ b/src/main/java/org/apache/commons/math4/random/RandomGenerator.java
@@ -18,8 +18,8 @@ package org.apache.commons.math4.random;
/**
- * Interface extracted from <code>java.util.Random</code>. This interface is
- * implemented by {@link AbstractRandomGenerator}.
+ * Interface extracted from <code>java.util.Random</code>.
+ * This interface is implemented by {@link BaseRandomGenerator}.
*
* @since 1.1
*/
http://git-wip-us.apache.org/repos/asf/commons-math/blob/aaabfe8d/src/main/java/org/apache/commons/math4/random/package-info.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/package-info.java b/src/main/java/org/apache/commons/math4/random/package-info.java
index 4d42815..0eec009 100644
--- a/src/main/java/org/apache/commons/math4/random/package-info.java
+++ b/src/main/java/org/apache/commons/math4/random/package-info.java
@@ -17,25 +17,29 @@
/**
*
* <p>Random number and random data generators.</p>
- * <p>Commons-math provides a few pseudo random number generators. The top level interface is RandomGenerator.
- * It is implemented by three classes:
- * <ul>
- * <li>{@link org.apache.commons.math4.random.JDKRandomGenerator JDKRandomGenerator}
- * that extends the JDK provided generator</li>
- * <li>AbstractRandomGenerator as a helper for users generators</li>
- * <li>BitStreamGenerator which is an abstract class for several generators and
- * which in turn is extended by:
+ * <p>
+ * Commons Math provides a few pseudo-random number generators.
+ * The top level interface is {@link RandomGenerator}.
+ * It is implemented by the following classes:
+ * <ul>
+ * <li>
+ * {@link org.apache.commons.math4.random.JDKRandomGenerator JDKRandomGenerator}
+ * that delegates to the JDK provided generator
+ * </li>
+ * <li>
+ * BaseRandomGenerator as a base class for concrete generators:
* <ul>
- * <li>{@link org.apache.commons.math4.random.MersenneTwister MersenneTwister}</li>
+ * <li>{@link org.apache.commons.math4.random.ISAACRandom ISAAC}</li>
* <li>{@link org.apache.commons.math4.random.Well512a Well512a}</li>
* <li>{@link org.apache.commons.math4.random.Well1024a Well1024a}</li>
* <li>{@link org.apache.commons.math4.random.Well19937a Well19937a}</li>
* <li>{@link org.apache.commons.math4.random.Well19937c Well19937c}</li>
* <li>{@link org.apache.commons.math4.random.Well44497a Well44497a}</li>
* <li>{@link org.apache.commons.math4.random.Well44497b Well44497b}</li>
+ * <li>{@link org.apache.commons.math4.random.MersenneTwister MersenneTwister}</li>
* </ul>
- * </li>
- * </ul>
+ * </li>
+ * </ul>
* </p>
*
* <p>
[11/18] [math] MATH-1309
Posted by er...@apache.org.
MATH-1309
Not calling public "setSeed" from a constructor.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/4fc5b340
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/4fc5b340
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/4fc5b340
Branch: refs/heads/master
Commit: 4fc5b3402c58d6a4b317bf23b896ea91d22af6fe
Parents: e0d17fe
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 17:16:33 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 17:33:00 2015 +0100
----------------------------------------------------------------------
.../commons/math4/random/MersenneTwister.java | 41 +++++++++++++-------
1 file changed, 28 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/4fc5b340/src/main/java/org/apache/commons/math4/random/MersenneTwister.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/MersenneTwister.java b/src/main/java/org/apache/commons/math4/random/MersenneTwister.java
index 709c343..1228b4e 100644
--- a/src/main/java/org/apache/commons/math4/random/MersenneTwister.java
+++ b/src/main/java/org/apache/commons/math4/random/MersenneTwister.java
@@ -111,7 +111,7 @@ public class MersenneTwister
*/
public MersenneTwister() {
mt = new int[N];
- setSeed(System.currentTimeMillis() + System.identityHashCode(this));
+ setSeedInternal(System.currentTimeMillis() + System.identityHashCode(this));
}
/** Creates a new random number generator using a single int seed.
@@ -119,7 +119,7 @@ public class MersenneTwister
*/
public MersenneTwister(int seed) {
mt = new int[N];
- setSeed(seed);
+ setSeedInternal(seed);
}
/** Creates a new random number generator using an int array seed.
@@ -128,7 +128,7 @@ public class MersenneTwister
*/
public MersenneTwister(int[] seed) {
mt = new int[N];
- setSeed(seed);
+ setSeedInternal(seed);
}
/** Creates a new random number generator using a single long seed.
@@ -136,7 +136,25 @@ public class MersenneTwister
*/
public MersenneTwister(long seed) {
mt = new int[N];
- setSeed(seed);
+ setSeedInternal(seed);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void setSeed(int seed) {
+ setSeedInternal(seed);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void setSeed(int[] seed) {
+ setSeedInternal(seed);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void setSeed(long seed) {
+ setSeedInternal(seed);
}
/** Reinitialize the generator as if just built with the given int seed.
@@ -144,8 +162,7 @@ public class MersenneTwister
* generator built with the same seed.</p>
* @param seed the initial seed (32 bits integer)
*/
- @Override
- public void setSeed(int seed) {
+ private void setSeedInternal(int seed) {
// we use a long masked by 0xffffffffL as a poor man unsigned int
long longMT = seed;
// NB: unlike original C code, we are working with java longs, the cast below makes masking unnecessary
@@ -167,15 +184,14 @@ public class MersenneTwister
* the seed of the generator will be the current system time plus the
* system identity hash code of this instance
*/
- @Override
- public void setSeed(int[] seed) {
+ private void setSeedInternal(int[] seed) {
if (seed == null) {
- setSeed(System.currentTimeMillis() + System.identityHashCode(this));
+ setSeedInternal(System.currentTimeMillis() + System.identityHashCode(this));
return;
}
- setSeed(19650218);
+ setSeedInternal(19650218);
int i = 1;
int j = 0;
@@ -217,9 +233,8 @@ public class MersenneTwister
* generator built with the same seed.</p>
* @param seed the initial seed (64 bits integer)
*/
- @Override
- public void setSeed(long seed) {
- setSeed(new int[] { (int) (seed >>> 32), (int) (seed & 0xffffffffl) });
+ private void setSeedInternal(long seed) {
+ setSeedInternal(new int[] { (int) (seed >>> 32), (int) (seed & 0xffffffffl) });
}
/**
[16/18] [math] MATH-1308
Posted by er...@apache.org.
MATH-1308
Removed obsolete and unused classes.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/8a35ca4f
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/8a35ca4f
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/8a35ca4f
Branch: refs/heads/master
Commit: 8a35ca4f8e19b914a8503d1d5156212363cbaf10
Parents: b95df5a
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 18:12:35 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 18:12:35 2015 +0100
----------------------------------------------------------------------
.../math4/random/AbstractRandomGenerator.java | 294 -------------------
.../random/AbstractRandomGeneratorTest.java | 39 ---
.../math4/random/TestRandomGenerator.java | 43 ---
3 files changed, 376 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/8a35ca4f/src/main/java/org/apache/commons/math4/random/AbstractRandomGenerator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/AbstractRandomGenerator.java b/src/main/java/org/apache/commons/math4/random/AbstractRandomGenerator.java
deleted file mode 100644
index 2115dbd..0000000
--- a/src/main/java/org/apache/commons/math4/random/AbstractRandomGenerator.java
+++ /dev/null
@@ -1,294 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.commons.math4.random;
-
-import org.apache.commons.math4.exception.NotStrictlyPositiveException;
-import org.apache.commons.math4.util.FastMath;
-
-/**
- * Abstract class implementing the {@link RandomGenerator} interface.
- * Default implementations for all methods other than {@link #nextDouble()} and
- * {@link #setSeed(long)} are provided.
- * <p>
- * All data generation methods are based on {@code code nextDouble()}.
- * Concrete implementations <strong>must</strong> override
- * this method and <strong>should</strong> provide better / more
- * performant implementations of the other methods if the underlying PRNG
- * supplies them.</p>
- *
- * @since 1.1
- */
-public abstract class AbstractRandomGenerator implements RandomGenerator {
-
- /**
- * Cached random normal value. The default implementation for
- * {@link #nextGaussian} generates pairs of values and this field caches the
- * second value so that the full algorithm is not executed for every
- * activation. The value {@code Double.NaN} signals that there is
- * no cached value. Use {@link #clear} to clear the cached value.
- */
- private double cachedNormalDeviate = Double.NaN;
-
- /**
- * Construct a RandomGenerator.
- */
- public AbstractRandomGenerator() {
- super();
-
- }
-
- /**
- * Clears the cache used by the default implementation of
- * {@link #nextGaussian}. Implementations that do not override the
- * default implementation of {@code nextGaussian} should call this
- * method in the implementation of {@link #setSeed(long)}
- */
- public void clear() {
- cachedNormalDeviate = Double.NaN;
- }
-
- /** {@inheritDoc} */
- @Override
- public void setSeed(int seed) {
- setSeed((long) seed);
- }
-
- /** {@inheritDoc} */
- @Override
- public void setSeed(int[] seed) {
- // the following number is the largest prime that fits in 32 bits (it is 2^32 - 5)
- final long prime = 4294967291l;
-
- long combined = 0l;
- for (int s : seed) {
- combined = combined * prime + s;
- }
- setSeed(combined);
- }
-
- /**
- * Sets the seed of the underlying random number generator using a
- * {@code long} seed. Sequences of values generated starting with the
- * same seeds should be identical.
- * <p>
- * Implementations that do not override the default implementation of
- * {@code nextGaussian} should include a call to {@link #clear} in the
- * implementation of this method.</p>
- *
- * @param seed the seed value
- */
- @Override
- public abstract void setSeed(long seed);
-
- /**
- * Generates random bytes and places them into a user-supplied
- * byte array. The number of random bytes produced is equal to
- * the length of the byte array.
- * <p>
- * The default implementation fills the array with bytes extracted from
- * random integers generated using {@link #nextInt}.</p>
- *
- * @param bytes the non-null byte array in which to put the
- * random bytes
- */
- @Override
- public void nextBytes(byte[] bytes) {
- // Multiple 4 part of length (i.e. length with two least significant bits unset).
- final int max = bytes.length & 0x7ffffffc;
-
- int index = 0;
- // Start filling in the byte array, 4 bytes at a time.
- while (index < max) {
- final int random = nextInt();
- bytes[index++] = (byte) random;
- bytes[index++] = (byte) (random >>> 8);
- bytes[index++] = (byte) (random >>> 16);
- bytes[index++] = (byte) (random >>> 24);
- }
-
- // Fill in the remaining bytes.
- if (index < bytes.length) {
- int random = nextInt();
- while (true) {
- bytes[index++] = (byte) random;
- if (index < bytes.length) {
- random >>>= 8;
- } else {
- break;
- }
- }
- }
- }
-
- /**
- * Returns the next pseudorandom, uniformly distributed {@code int}
- * value from this random number generator's sequence.
- * All 2<font size="-1"><sup>32</sup></font> possible {@code int} values
- * should be produced with (approximately) equal probability.
- * <p>
- * The default implementation provided here returns
- * <pre>
- * <code>(int) (nextDouble() * Integer.MAX_VALUE)</code>
- * </pre></p>
- *
- * @return the next pseudorandom, uniformly distributed {@code int}
- * value from this random number generator's sequence
- */
- @Override
- public int nextInt() {
- return (int) ((2d * nextDouble() - 1d) * Integer.MAX_VALUE);
- }
-
- /**
- * Returns a pseudorandom, uniformly distributed {@code int} value
- * between 0 (inclusive) and the specified value (exclusive), drawn from
- * this random number generator's sequence.
- * <p>
- * The default implementation returns
- * <pre>
- * <code>(int) (nextDouble() * n</code>
- * </pre></p>
- *
- * @param n the bound on the random number to be returned. Must be
- * positive.
- * @return a pseudorandom, uniformly distributed {@code int}
- * value between 0 (inclusive) and n (exclusive).
- * @throws NotStrictlyPositiveException if {@code n <= 0}.
- */
- @Override
- public int nextInt(int n) {
- if (n <= 0 ) {
- throw new NotStrictlyPositiveException(n);
- }
- int result = (int) (nextDouble() * n);
- return result < n ? result : n - 1;
- }
-
- /**
- * Returns the next pseudorandom, uniformly distributed {@code long}
- * value from this random number generator's sequence. All
- * 2<font size="-1"><sup>64</sup></font> possible {@code long} values
- * should be produced with (approximately) equal probability.
- * <p>
- * The default implementation returns
- * <pre>
- * <code>(long) (nextDouble() * Long.MAX_VALUE)</code>
- * </pre></p>
- *
- * @return the next pseudorandom, uniformly distributed {@code long}
- *value from this random number generator's sequence
- */
- @Override
- public long nextLong() {
- return (long) ((2d * nextDouble() - 1d) * Long.MAX_VALUE);
- }
-
- /**
- * Returns the next pseudorandom, uniformly distributed
- * {@code boolean} value from this random number generator's
- * sequence.
- * <p>
- * The default implementation returns
- * <pre>
- * <code>nextDouble() <= 0.5</code>
- * </pre></p>
- *
- * @return the next pseudorandom, uniformly distributed
- * {@code boolean} value from this random number generator's
- * sequence
- */
- @Override
- public boolean nextBoolean() {
- return nextDouble() <= 0.5;
- }
-
- /**
- * Returns the next pseudorandom, uniformly distributed {@code float}
- * value between {@code 0.0} and {@code 1.0} from this random
- * number generator's sequence.
- * <p>
- * The default implementation returns
- * <pre>
- * <code>(float) nextDouble() </code>
- * </pre></p>
- *
- * @return the next pseudorandom, uniformly distributed {@code float}
- * value between {@code 0.0} and {@code 1.0} from this
- * random number generator's sequence
- */
- @Override
- public float nextFloat() {
- return (float) nextDouble();
- }
-
- /**
- * Returns the next pseudorandom, uniformly distributed
- * {@code double} value between {@code 0.0} and
- * {@code 1.0} from this random number generator's sequence.
- * <p>
- * This method provides the underlying source of random data used by the
- * other methods.</p>
- *
- * @return the next pseudorandom, uniformly distributed
- * {@code double} value between {@code 0.0} and
- * {@code 1.0} from this random number generator's sequence
- */
- @Override
- public abstract double nextDouble();
-
- /**
- * Returns the next pseudorandom, Gaussian ("normally") distributed
- * {@code double} value with mean {@code 0.0} and standard
- * deviation {@code 1.0} from this random number generator's sequence.
- * <p>
- * The default implementation uses the <em>Polar Method</em>
- * due to G.E.P. Box, M.E. Muller and G. Marsaglia, as described in
- * D. Knuth, <u>The Art of Computer Programming</u>, 3.4.1C.</p>
- * <p>
- * The algorithm generates a pair of independent random values. One of
- * these is cached for reuse, so the full algorithm is not executed on each
- * activation. Implementations that do not override this method should
- * make sure to call {@link #clear} to clear the cached value in the
- * implementation of {@link #setSeed(long)}.</p>
- *
- * @return the next pseudorandom, Gaussian ("normally") distributed
- * {@code double} value with mean {@code 0.0} and
- * standard deviation {@code 1.0} from this random number
- * generator's sequence
- */
- @Override
- public double nextGaussian() {
- if (!Double.isNaN(cachedNormalDeviate)) {
- double dev = cachedNormalDeviate;
- cachedNormalDeviate = Double.NaN;
- return dev;
- }
- double v1 = 0;
- double v2 = 0;
- double s = 1;
- while (s >=1 ) {
- v1 = 2 * nextDouble() - 1;
- v2 = 2 * nextDouble() - 1;
- s = v1 * v1 + v2 * v2;
- }
- if (s != 0) {
- s = FastMath.sqrt(-2 * FastMath.log(s) / s);
- }
- cachedNormalDeviate = v2 * s;
- return v1 * s;
- }
-}
http://git-wip-us.apache.org/repos/asf/commons-math/blob/8a35ca4f/src/test/java/org/apache/commons/math4/random/AbstractRandomGeneratorTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/AbstractRandomGeneratorTest.java b/src/test/java/org/apache/commons/math4/random/AbstractRandomGeneratorTest.java
deleted file mode 100644
index 79cf97d..0000000
--- a/src/test/java/org/apache/commons/math4/random/AbstractRandomGeneratorTest.java
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.commons.math4.random;
-
-import org.apache.commons.math4.random.RandomGenerator;
-
-/**
- * Test cases for the AbstractRandomGenerator class.
- *
- */
-
-public class AbstractRandomGeneratorTest extends RandomGeneratorAbstractTest {
-
- public AbstractRandomGeneratorTest() {
- super();
- }
-
- @Override
- protected RandomGenerator makeGenerator() {
- RandomGenerator generator = new TestRandomGenerator();
- generator.setSeed(1001);
- return generator;
- }
-
-}
http://git-wip-us.apache.org/repos/asf/commons-math/blob/8a35ca4f/src/test/java/org/apache/commons/math4/random/TestRandomGenerator.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/TestRandomGenerator.java b/src/test/java/org/apache/commons/math4/random/TestRandomGenerator.java
deleted file mode 100644
index fc820b2..0000000
--- a/src/test/java/org/apache/commons/math4/random/TestRandomGenerator.java
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.commons.math4.random;
-import java.util.Random;
-
-import org.apache.commons.math4.random.AbstractRandomGenerator;
-
-/**
- * Dummy AbstractRandomGenerator concrete subclass that just wraps a
- * java.util.Random instance. Used by AbstractRandomGeneratorTest to test
- * default implementations in AbstractRandomGenerator.
- *
- */
-public class TestRandomGenerator extends AbstractRandomGenerator {
-
- private Random random = new Random();
-
- @Override
- public void setSeed(long seed) {
- clear();
- random.setSeed(seed);
- }
-
- @Override
- public double nextDouble() {
- return random.nextDouble();
- }
-
-}
[18/18] [math] Update "changes.xml".
Posted by er...@apache.org.
Update "changes.xml".
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/81585a3c
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/81585a3c
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/81585a3c
Branch: refs/heads/master
Commit: 81585a3c46bb05e86e11d88ba25d14b90d488577
Parents: 8d6d088
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 18:22:08 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 18:22:08 2015 +0100
----------------------------------------------------------------------
src/changes/changes.xml | 15 +++++++++++++++
1 file changed, 15 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/81585a3c/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 20005d4..b500dd2 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -54,6 +54,21 @@ If the output is not quite correct, check for invisible trailing spaces!
</release>
<release version="4.0" date="XXXX-XX-XX" description="">
+ <action dev="erans" type="fix" issue="MATH-1308">
+ Removed obsolete class "AbstractRandomGenerator" (package "o.a.c.m.random").
+ </action>
+ <action dev="erans" type="fix" issue="MATH-1306" due-to="Rostislav Krasny">
+ New method "nextBytes(byte[],int,int)" in "BaseRandomGenerator"
+ for partial filling of the user-supplied array.
+ </action>
+ <action dev="erans" type="fix" issue="MATH-1309">
+ Remove calls to public methods "setSeed" from constructors (in RNG implementations).
+ </action>
+ <action dev="erans" type="fix" issue="MATH-1307">
+ New base class for all RNG implementations: "BaseRandomGenerator" replaces
+ "BitsStreamGenerator" whose method "nextInt(int)" is replaced by "nextInt()"
+ as the generator of randomness that must be implemented in concrete subclasses.
+ </action>
<action dev="luc" type="fix" issue="MATH-1297"> <!-- backported to 3.6 -->
Detect start failures with multi-step ODE integrators.
</action>
[17/18] [math] Checkstyle.
Posted by er...@apache.org.
Checkstyle.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/8d6d0886
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/8d6d0886
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/8d6d0886
Branch: refs/heads/master
Commit: 8d6d088622f8e4ea89e66ccbe097510fb4c932fd
Parents: 8a35ca4
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 18:18:25 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 18:18:25 2015 +0100
----------------------------------------------------------------------
.../java/org/apache/commons/math4/random/BaseRandomGenerator.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/8d6d0886/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java b/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
index 2d78005..da801b0 100644
--- a/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
+++ b/src/main/java/org/apache/commons/math4/random/BaseRandomGenerator.java
@@ -120,7 +120,7 @@ public abstract class BaseRandomGenerator
int bits;
int val;
do {
- bits = (nextInt() >>> 1);
+ bits = nextInt() >>> 1;
val = bits % n;
} while (bits - val + (n - 1) < 0);
return val;
[10/18] [math] MATH-1309
Posted by er...@apache.org.
MATH-1309
Not calling public "setSeed" from a constructor.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/e0d17fed
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/e0d17fed
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/e0d17fed
Branch: refs/heads/master
Commit: e0d17fed51f5018fc34578699d824c20efc44368
Parents: 581b474
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 17:11:31 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 17:32:56 2015 +0100
----------------------------------------------------------------------
.../commons/math4/random/ISAACRandom.java | 41 ++++++++++++++++----
1 file changed, 34 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/e0d17fed/src/main/java/org/apache/commons/math4/random/ISAACRandom.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/ISAACRandom.java b/src/main/java/org/apache/commons/math4/random/ISAACRandom.java
index b9c0e36..185710f 100644
--- a/src/main/java/org/apache/commons/math4/random/ISAACRandom.java
+++ b/src/main/java/org/apache/commons/math4/random/ISAACRandom.java
@@ -85,7 +85,7 @@ public class ISAACRandom
* current time and system hash code of the instance as the seed.
*/
public ISAACRandom() {
- setSeed(System.currentTimeMillis() + System.identityHashCode(this));
+ setSeedInternal(System.currentTimeMillis() + System.identityHashCode(this));
}
/**
@@ -94,7 +94,7 @@ public class ISAACRandom
* @param seed Initial seed.
*/
public ISAACRandom(long seed) {
- setSeed(seed);
+ setSeedInternal(seed);
}
/**
@@ -104,24 +104,51 @@ public class ISAACRandom
* to the current time.
*/
public ISAACRandom(int[] seed) {
- setSeed(seed);
+ setSeedInternal(seed);
}
/** {@inheritDoc} */
@Override
public void setSeed(int seed) {
- setSeed(new int[]{seed});
+ setSeedInternal(seed);
}
/** {@inheritDoc} */
@Override
- public void setSeed(long seed) {
- setSeed(new int[]{(int) (seed >>> 32), (int) (seed & 0xffffffffL)});
+ public void setSeed(int[] seed) {
+ setSeedInternal(seed);
}
/** {@inheritDoc} */
@Override
- public void setSeed(int[] seed) {
+ public void setSeed(long seed) {
+ setSeedInternal(seed);
+ }
+
+ /**
+ * Reseeds the RNG.
+ *
+ * @param seed Seed.
+ */
+ private void setSeedInternal(int seed) {
+ setSeed(new int[]{seed});
+ }
+
+ /**
+ * Reseeds the RNG.
+ *
+ * @param seed Seed.
+ */
+ private void setSeedInternal(long seed) {
+ setSeed(new int[]{(int) (seed >>> 32), (int) (seed & 0xffffffffL)});
+ }
+
+ /**
+ * Reseeds the RNG.
+ *
+ * @param seed Seed.
+ */
+ private void setSeedInternal(int[] seed) {
if (seed == null) {
setSeed(System.currentTimeMillis() + System.identityHashCode(this));
return;
[04/18] [math] MATH-1307
Posted by er...@apache.org.
MATH-1307
Extends new base class.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/87497c72
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/87497c72
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/87497c72
Branch: refs/heads/master
Commit: 87497c72460c9c58f829a372d741b386fc2ac8f8
Parents: 6f4f676
Author: Gilles <er...@apache.org>
Authored: Mon Dec 28 16:47:52 2015 +0100
Committer: Gilles <er...@apache.org>
Committed: Mon Dec 28 16:47:52 2015 +0100
----------------------------------------------------------------------
.../commons/math4/random/MersenneTwister.java | 16 ++++++++--------
.../commons/math4/random/MersenneTwisterTest.java | 2 +-
2 files changed, 9 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/87497c72/src/main/java/org/apache/commons/math4/random/MersenneTwister.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/random/MersenneTwister.java b/src/main/java/org/apache/commons/math4/random/MersenneTwister.java
index c84ff8c..d8caaa7 100644
--- a/src/main/java/org/apache/commons/math4/random/MersenneTwister.java
+++ b/src/main/java/org/apache/commons/math4/random/MersenneTwister.java
@@ -83,10 +83,12 @@ import org.apache.commons.math4.util.FastMath;
* @since 2.0
*/
-public class MersenneTwister extends BitsStreamGenerator implements Serializable {
+public class MersenneTwister
+ extends BaseRandomGenerator
+ implements Serializable {
/** Serializable version identifier. */
- private static final long serialVersionUID = 8661194735290153518L;
+ private static final long serialVersionUID = 20151228L;
/** Size of the bytes pool. */
private static final int N = 624;
@@ -224,13 +226,13 @@ public class MersenneTwister extends BitsStreamGenerator implements Serializable
* <p>This method is the core generation algorithm. It is used by all the
* public generation methods for the various primitive types {@link
* #nextBoolean()}, {@link #nextBytes(byte[])}, {@link #nextDouble()},
- * {@link #nextFloat()}, {@link #nextGaussian()}, {@link #nextInt()},
- * {@link #next(int)} and {@link #nextLong()}.</p>
+ * {@link #nextFloat()}, {@link #nextGaussian()} and {@link #nextLong()}.
+ * </p>
* @param bits number of random bits to produce
* @return random bits generated
*/
@Override
- protected int next(int bits) {
+ public int nextInt() {
int y;
@@ -262,8 +264,6 @@ public class MersenneTwister extends BitsStreamGenerator implements Serializable
y ^= (y << 15) & 0xefc60000;
y ^= y >>> 18;
- return y >>> (32 - bits);
-
+ return y;
}
-
}
http://git-wip-us.apache.org/repos/asf/commons-math/blob/87497c72/src/test/java/org/apache/commons/math4/random/MersenneTwisterTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/random/MersenneTwisterTest.java b/src/test/java/org/apache/commons/math4/random/MersenneTwisterTest.java
index 1446003..7c55d6a 100644
--- a/src/test/java/org/apache/commons/math4/random/MersenneTwisterTest.java
+++ b/src/test/java/org/apache/commons/math4/random/MersenneTwisterTest.java
@@ -21,7 +21,7 @@ import org.apache.commons.math4.random.RandomGenerator;
import org.junit.Assert;
import org.junit.Test;
-public class MersenneTwisterTest extends RandomGeneratorAbstractTest {
+public class MersenneTwisterTest extends BaseRandomGeneratorTest {
@Override
protected RandomGenerator makeGenerator() {