Fixed rounding error in RandomDataImpl nextInt, nextLong methods causing lower

endpoints to be excluded when negative. Also improved robustness of nextUniform
for extreme values and changed its contract to throw IAE when provided bounds
are infinite or NaN.

JIRA: MATH-724
Reported and patched by Dennis Hendriks




git-svn-id: https://svn.apache.org/repos/asf/commons/proper/math/trunk@1221490 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Phil Steitz 2011-12-20 21:07:58 +00:00
parent 0093f7b0a2
commit 9c8bb93443
6 changed files with 312 additions and 108 deletions

View File

@ -116,6 +116,7 @@ public enum LocalizedFormats implements Localizable {
INDEX_OUT_OF_RANGE("index {0} out of allowed range [{1}, {2}]"),
INDEX("index ({0})"), /* keep */
NOT_FINITE_NUMBER("{0} is not a finite number"), /* keep */
INFINITE_BOUND("interval bounds must be finite"),
ARRAY_ELEMENT("value {0} at index {1}"), /* keep */
INFINITE_ARRAY_ELEMENT("Array contains an infinite element, {0} at index {1}"),
INFINITE_VALUE_CONVERSION("cannot convert infinite value"),
@ -240,6 +241,7 @@ public enum LocalizedFormats implements Localizable {
NO_REGRESSORS("Regression model must include at least one regressor"),
NO_RESULT_AVAILABLE("no result available"),
NO_SUCH_MATRIX_ENTRY("no entry at indices ({0}, {1}) in a {2}x{3} matrix"),
NAN_NOT_ALLOWED("NaN is not allowed"),
NULL_NOT_ALLOWED("null is not allowed"), /* keep */
ARRAY_ZERO_LENGTH_OR_NULL_NOTALLOWED("A null or zero length array not allowed"),
COVARIANCE_MATRIX("covariance matrix"), /* keep */

View File

@ -36,6 +36,7 @@ import org.apache.commons.math.distribution.PascalDistribution;
import org.apache.commons.math.distribution.TDistribution;
import org.apache.commons.math.distribution.WeibullDistribution;
import org.apache.commons.math.distribution.ZipfDistribution;
import org.apache.commons.math.exception.MathIllegalArgumentException;
import org.apache.commons.math.exception.MathInternalError;
import org.apache.commons.math.exception.NotStrictlyPositiveException;
import org.apache.commons.math.exception.NumberIsTooLargeException;
@ -250,7 +251,8 @@ public class RandomDataImpl implements RandomData, Serializable {
lower, upper, false);
}
double r = getRan().nextDouble();
return (int) ((r * upper) + ((1.0 - r) * lower) + r);
double scaled = r * upper + (1.0 - r) * lower + r;
return (int)FastMath.floor(scaled);
}
/**
@ -270,7 +272,8 @@ public class RandomDataImpl implements RandomData, Serializable {
lower, upper, false);
}
double r = getRan().nextDouble();
return (long) ((r * upper) + ((1.0 - r) * lower) + r);
double scaled = r * upper + (1.0 - r) * lower + r;
return (long)FastMath.floor(scaled);
}
/**
@ -361,7 +364,9 @@ public class RandomDataImpl implements RandomData, Serializable {
lower, upper, false);
}
SecureRandom sec = getSecRan();
return lower + (int) (sec.nextDouble() * (upper - lower + 1));
double r = sec.nextDouble();
double scaled = r * upper + (1.0 - r) * lower + r;
return (int)FastMath.floor(scaled);
}
/**
@ -382,7 +387,9 @@ public class RandomDataImpl implements RandomData, Serializable {
lower, upper, false);
}
SecureRandom sec = getSecRan();
return lower + (long) (sec.nextDouble() * (upper - lower + 1));
double r = sec.nextDouble();
double scaled = r * upper + (1.0 - r) * lower + r;
return (long)FastMath.floor(scaled);
}
/**
@ -579,19 +586,26 @@ public class RandomDataImpl implements RandomData, Serializable {
* provide a symmetric output interval (both endpoints excluded).
* </p>
*
* @param lower
* the lower bound.
* @param upper
* the upper bound.
* @return a uniformly distributed random value from the interval (lower,
* upper)
* @throws NumberIsTooLargeException if {@code lower >= upper}.
* @param lower the lower bound.
* @param upper the upper bound.
* @return a uniformly distributed random value from the interval (lower, upper)
* @throws MathIllegalArgumentException if {@code lower >= upper}
* or either bound is infinite or NaN
*/
public double nextUniform(double lower, double upper) {
if (lower >= upper) {
throw new NumberIsTooLargeException(LocalizedFormats.LOWER_BOUND_NOT_BELOW_UPPER_BOUND,
lower, upper, false);
throw new MathIllegalArgumentException(LocalizedFormats.LOWER_BOUND_NOT_BELOW_UPPER_BOUND,
lower, upper);
}
if (Double.isInfinite(lower) || Double.isInfinite(upper)) {
throw new MathIllegalArgumentException(LocalizedFormats.INFINITE_BOUND);
}
if (Double.isNaN(lower) || Double.isNaN(upper)) {
throw new MathIllegalArgumentException(LocalizedFormats.NAN_NOT_ALLOWED);
}
final RandomGenerator generator = getRan();
// ensure nextDouble() isn't 0.0
@ -600,7 +614,7 @@ public class RandomDataImpl implements RandomData, Serializable {
u = generator.nextDouble();
}
return lower + u * (upper - lower);
return u * upper + (1.0 - u) * lower;
}
/**

View File

@ -86,6 +86,7 @@ INDEX_NOT_POSITIVE = l''indice ({0}) n''est pas positif
INDEX_OUT_OF_RANGE = l''indice ({0}) est hors du domaine autoris\u00e9 [{1}, {2}]
INDEX = indice ({0})
NOT_FINITE_NUMBER = {0} n''est pas un nombre fini
INFINITE_BOUND = intervalle limites doit \u00eatre finie
ARRAY_ELEMENT = valeur {0} à l''indice {1}
INFINITE_ARRAY_ELEMENT = le tableau contient l''\u00e9l\u00e9ment infini {0} \u00e0 l''index {1}
INFINITE_VALUE_CONVERSION = les valeurs infinies ne peuvent \u00eatre converties
@ -208,6 +209,7 @@ NO_OPTIMUM_COMPUTED_YET = aucun optimum n''a encore \u00e9t\u00e9 calcul\u00e9
NO_REGRESSORS = le mod\u00e8le de r\u00e9gression doit inclure au moins une variable explicative
NO_RESULT_AVAILABLE = aucun r\u00e9sultat n''est disponible
NO_SUCH_MATRIX_ENTRY = pas d''\u00e9l\u00e9ment ({0}, {1}) dans une matrice {2}x{3}
NAN_NOT_ALLOWED = "NaN" n''est pas permis
NULL_NOT_ALLOWED = "null" n''est pas permis
COVARIANCE_MATRIX = matrice de covariance
DENOMINATOR = d\u00e9nominateur

View File

@ -52,6 +52,12 @@ The <action> type attribute can be add,update,fix,remove.
If the output is not quite correct, check for invisible trailing spaces!
-->
<release version="3.0" date="TBD" description="TBD">
<action dev="psteitz" type="fixe" issue="MATH-724" due-to="Dennis Hendricks">
Fixed rounding error in RandomDataImpl nextInt, nextLong methods causing lower
endpoints to be excluded when negative. Also improved robustness of nextUniform
for extreme values and changed its contract to throw IAE when provided bounds
are infinite or NaN.
</action>
<action dev="erans" type="fix" issue="MATH-728" due-to="Bruce A. Johnson">
Fixed "offset by one" bug in "BOBYQAOptimizer".
</action>

View File

@ -88,122 +88,236 @@ public class RandomDataTest {
long y = randomData.nextLong(Long.MIN_VALUE, Long.MAX_VALUE);
Assert.assertFalse(x == y);
}
/** test dispersion and failure modes for nextInt() */
@Test
public void testNextInt() {
public void testNextUniformExtremeValues() {
double x = randomData.nextUniform(-Double.MAX_VALUE, Double.MAX_VALUE);
double y = randomData.nextUniform(-Double.MAX_VALUE, Double.MAX_VALUE);
Assert.assertFalse(x == y);
Assert.assertFalse(Double.isNaN(x));
Assert.assertFalse(Double.isNaN(y));
Assert.assertFalse(Double.isInfinite(x));
Assert.assertFalse(Double.isInfinite(y));
}
@Test
public void testNextIntIAE() throws Exception {
try {
randomData.nextInt(4, 3);
Assert.fail("MathIllegalArgumentException expected");
} catch (MathIllegalArgumentException ex) {
// ignored
}
Frequency freq = new Frequency();
int value = 0;
}
@Test
public void testNextIntNegativeToPositiveRange() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextIntUniform(-3, 5);
checkNextIntUniform(-3, 6);
}
}
@Test
public void testNextIntNegativeRange() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextIntUniform(-7, -4);
checkNextIntUniform(-15, -2);
}
}
@Test
public void testNextIntPositiveRange() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextIntUniform(0, 3);
checkNextIntUniform(2, 12);
checkNextIntUniform(1,2);
}
}
private void checkNextIntUniform(int min, int max) throws Exception {
final Frequency freq = new Frequency();
for (int i = 0; i < smallSampleSize; i++) {
value = randomData.nextInt(0, 3);
Assert.assertTrue("nextInt range", (value >= 0) && (value <= 3));
final int value = randomData.nextInt(min, max);
Assert.assertTrue("nextInt range", (value >= min) && (value <= max));
freq.addValue(value);
}
long[] observed = new long[4];
for (int i = 0; i < 4; i++) {
observed[i] = freq.getCount(i);
final int len = max - min + 1;
final long[] observed = new long[len];
for (int i = 0; i < len; i++) {
observed[i] = freq.getCount(min + i);
}
/*
* Use ChiSquare dist with df = 4-1 = 3, alpha = .001 Change to 11.34
* for alpha = .01
*/
Assert.assertTrue("chi-square test -- will fail about 1 in 1000 times",
testStatistic.chiSquare(expected, observed) < 16.27);
final double[] expected = new double[len];
for (int i = 0; i < len; i++) {
expected[i] = 1d / len;
}
TestUtils.assertChiSquareAccept(expected, observed, 0.01);
}
/** test dispersion and failure modes for nextLong() */
@Test
public void testNextLong() {
public void testNextLongIAE() {
try {
randomData.nextLong(4, 3);
Assert.fail("MathIllegalArgumentException expected");
} catch (MathIllegalArgumentException ex) {
// ignored
}
Frequency freq = new Frequency();
long value = 0;
}
@Test
public void testNextLongNegativeToPositiveRange() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextLongUniform(-3, 5);
checkNextLongUniform(-3, 6);
}
}
@Test
public void testNextLongNegativeRange() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextLongUniform(-7, -4);
checkNextLongUniform(-15, -2);
}
}
@Test
public void testNextLongPositiveRange() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextLongUniform(0, 3);
checkNextLongUniform(2, 12);
}
}
private void checkNextLongUniform(int min, int max) throws Exception {
final Frequency freq = new Frequency();
for (int i = 0; i < smallSampleSize; i++) {
value = randomData.nextLong(0, 3);
Assert.assertTrue("nextInt range", (value >= 0) && (value <= 3));
final long value = randomData.nextLong(min, max);
Assert.assertTrue("nextLong range", (value >= min) && (value <= max));
freq.addValue(value);
}
long[] observed = new long[4];
for (int i = 0; i < 4; i++) {
observed[i] = freq.getCount(i);
final int len = max - min + 1;
final long[] observed = new long[len];
for (int i = 0; i < len; i++) {
observed[i] = freq.getCount(min + i);
}
/*
* Use ChiSquare dist with df = 4-1 = 3, alpha = .001 Change to 11.34
* for alpha = .01
*/
Assert.assertTrue("chi-square test -- will fail about 1 in 1000 times",
testStatistic.chiSquare(expected, observed) < 16.27);
final double[] expected = new double[len];
for (int i = 0; i < len; i++) {
expected[i] = 1d / len;
}
TestUtils.assertChiSquareAccept(expected, observed, 0.01);
}
/** test dispersion and failure modes for nextSecureLong() */
@Test
public void testNextSecureLong() {
public void testNextSecureLongIAE() {
try {
randomData.nextSecureLong(4, 3);
Assert.fail("MathIllegalArgumentException expected");
} catch (MathIllegalArgumentException ex) {
// ignored
}
Frequency freq = new Frequency();
long value = 0;
}
@Test
public void testNextSecureLongNegativeToPositiveRange() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextSecureLongUniform(-3, 5);
checkNextSecureLongUniform(-3, 6);
}
}
@Test
public void testNextSecureLongNegativeRange() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextSecureLongUniform(-7, -4);
checkNextSecureLongUniform(-15, -2);
}
}
@Test
public void testNextSecureLongPositiveRange() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextSecureLongUniform(0, 3);
checkNextSecureLongUniform(2, 12);
}
}
private void checkNextSecureLongUniform(int min, int max) throws Exception {
final Frequency freq = new Frequency();
for (int i = 0; i < smallSampleSize; i++) {
value = randomData.nextSecureLong(0, 3);
Assert.assertTrue("nextInt range", (value >= 0) && (value <= 3));
final long value = randomData.nextSecureLong(min, max);
Assert.assertTrue("nextLong range", (value >= min) && (value <= max));
freq.addValue(value);
}
long[] observed = new long[4];
for (int i = 0; i < 4; i++) {
observed[i] = freq.getCount(i);
final int len = max - min + 1;
final long[] observed = new long[len];
for (int i = 0; i < len; i++) {
observed[i] = freq.getCount(min + i);
}
/*
* Use ChiSquare dist with df = 4-1 = 3, alpha = .001 Change to 11.34
* for alpha = .01
*/
Assert.assertTrue("chi-square test -- will fail about 1 in 1000 times",
testStatistic.chiSquare(expected, observed) < 16.27);
final double[] expected = new double[len];
for (int i = 0; i < len; i++) {
expected[i] = 1d / len;
}
TestUtils.assertChiSquareAccept(expected, observed, 0.0001);
}
/** test dispersion and failure modes for nextSecureInt() */
@Test
public void testNextSecureInt() {
public void testNextSecureIntIAE() {
try {
randomData.nextSecureInt(4, 3);
Assert.fail("MathIllegalArgumentException expected");
} catch (MathIllegalArgumentException ex) {
// ignored
}
Frequency freq = new Frequency();
int value = 0;
}
@Test
public void testNextSecureIntNegativeToPositiveRange() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextSecureIntUniform(-3, 5);
checkNextSecureIntUniform(-3, 6);
}
}
@Test
public void testNextSecureIntNegativeRange() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextSecureIntUniform(-7, -4);
checkNextSecureIntUniform(-15, -2);
}
}
@Test
public void testNextSecureIntPositiveRange() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextSecureIntUniform(0, 3);
checkNextSecureIntUniform(2, 12);
}
}
private void checkNextSecureIntUniform(int min, int max) throws Exception {
final Frequency freq = new Frequency();
for (int i = 0; i < smallSampleSize; i++) {
value = randomData.nextSecureInt(0, 3);
Assert.assertTrue("nextInt range", (value >= 0) && (value <= 3));
final int value = randomData.nextSecureInt(min, max);
Assert.assertTrue("nextInt range", (value >= min) && (value <= max));
freq.addValue(value);
}
long[] observed = new long[4];
for (int i = 0; i < 4; i++) {
observed[i] = freq.getCount(i);
final int len = max - min + 1;
final long[] observed = new long[len];
for (int i = 0; i < len; i++) {
observed[i] = freq.getCount(min + i);
}
/*
* Use ChiSquare dist with df = 4-1 = 3, alpha = .001 Change to 11.34
* for alpha = .01
*/
Assert.assertTrue("chi-square test -- will fail about 1 in 1000 times",
testStatistic.chiSquare(expected, observed) < 16.27);
final double[] expected = new double[len];
for (int i = 0; i < len; i++) {
expected[i] = 1d / len;
}
TestUtils.assertChiSquareAccept(expected, observed, 0.0001);
}
/**
* Make sure that empirical distribution of random Poisson(4)'s has P(X <=
@ -505,9 +619,8 @@ public class RandomDataTest {
testStatistic.chiSquare(expected, observed) < 37.70);
}
/** test failure modes and dispersion of nextUniform() */
@Test
public void testNextUniform() {
public void testNextUniformIAE() {
try {
randomData.nextUniform(4, 3);
Assert.fail("MathIllegalArgumentException expected");
@ -515,34 +628,91 @@ public class RandomDataTest {
// ignored
}
try {
randomData.nextUniform(3, 3);
randomData.nextUniform(0, Double.POSITIVE_INFINITY);
Assert.fail("MathIllegalArgumentException expected");
} catch (MathIllegalArgumentException ex) {
// ignored
}
double[] expected = { 500, 500 };
long[] observed = { 0, 0 };
double lower = -1d;
double upper = 20d;
double midpoint = (lower + upper) / 2d;
double result = 0;
for (int i = 0; i < 1000; i++) {
result = randomData.nextUniform(lower, upper);
if ((result == lower) || (result == upper)) {
Assert.fail("generated value equal to an endpoint: " + result);
}
if (result < midpoint) {
observed[0]++;
} else {
observed[1]++;
}
try {
randomData.nextUniform(Double.NEGATIVE_INFINITY, 0);
Assert.fail("MathIllegalArgumentException expected");
} catch (MathIllegalArgumentException ex) {
// ignored
}
/*
* Use ChiSquare dist with df = 2-1 = 1, alpha = .001 Change to 6.64 for
* alpha = .01
*/
Assert.assertTrue("chi-square test -- will fail about 1 in 1000 times",
testStatistic.chiSquare(expected, observed) < 10.83);
try {
randomData.nextUniform(0, Double.NaN);
Assert.fail("MathIllegalArgumentException expected");
} catch (MathIllegalArgumentException ex) {
// ignored
}
try {
randomData.nextUniform(Double.NaN, 0);
Assert.fail("MathIllegalArgumentException expected");
} catch (MathIllegalArgumentException ex) {
// ignored
}
}
@Test
public void testNextUniformUniformPositiveBounds() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextUniformUniform(0, 10);
}
}
@Test
public void testNextUniformUniformNegativeToPositiveBounds() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextUniformUniform(-3, 5);
}
}
@Test
public void testNextUniformUniformNegaiveBounds() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextUniformUniform(-7, -3);
}
}
@Test
public void testNextUniformUniformMaximalInterval() throws Exception {
for (int i = 0; i < 5; i++) {
checkNextUniformUniform(-Double.MAX_VALUE, Double.MAX_VALUE);
}
}
private void checkNextUniformUniform(double min, double max) throws Exception {
// Set up bin bounds - min, binBound[0], ..., binBound[binCount-2], max
final int binCount = 5;
final double binSize = max / binCount - min/binCount; // Prevent overflow in extreme value case
final double[] binBounds = new double[binCount - 1];
binBounds[0] = min + binSize;
for (int i = 1; i < binCount - 1; i++) {
binBounds[i] = binBounds[i - 1] + binSize; // + instead of * to avoid overflow in extreme case
}
final Frequency freq = new Frequency();
for (int i = 0; i < smallSampleSize; i++) {
final double value = randomData.nextUniform(min, max);
Assert.assertTrue("nextUniform range", (value > min) && (value < max));
// Find bin
int j = 0;
while (j < binCount - 1 && value > binBounds[j]) {
j++;
}
freq.addValue(j);
}
final long[] observed = new long[binCount];
for (int i = 0; i < binCount; i++) {
observed[i] = freq.getCount(i);
}
final double[] expected = new double[binCount];
for (int i = 0; i < binCount; i++) {
expected[i] = 1d / binCount;
}
TestUtils.assertChiSquareAccept(expected, observed, 0.01);
}
/** test exclusive endpoints of nextUniform **/

View File

@ -70,11 +70,21 @@ public abstract class RandomGeneratorAbstractTest extends RandomDataTest {
// Omit secureXxx tests, since they do not use the provided generator
@Override
public void testNextSecureLong() {}
public void testNextSecureLongIAE() {}
@Override
public void testNextSecureInt() {}
public void testNextSecureLongNegativeToPositiveRange() {}
@Override
public void testNextSecureLongNegativeRange() {}
@Override
public void testNextSecureLongPositiveRange() {}
@Override
public void testNextSecureIntIAE() {}
@Override
public void testNextSecureIntNegativeToPositiveRange() {}
@Override
public void testNextSecureIntNegativeRange() {}
@Override
public void testNextSecureIntPositiveRange() {}
@Override
public void testNextSecureHex() {}