On 12/14/12 9:43 AM, Phil Steitz wrote: > On 12/14/12 8:52 AM, Gilles Sadowski wrote: >> On Fri, Dec 14, 2012 at 04:28:25PM -0000, pste...@apache.org wrote: >>> Author: psteitz >>> Date: Fri Dec 14 16:28:23 2012 >>> New Revision: 1421968 >>> >>> URL: http://svn.apache.org/viewvc?rev=1421968&view=rev >>> Log: >>> Reverted incompatible changes made in r1420006. >> I don't understand why you did that. >> You nullified my changes that fixed (IIUC) the problem while still >> providing users a smooth upgrade path to using RandomDataGenerator >> (which it seems was your goal). > I was basically reverting my original commit, which caused the > problem. I started with a revert back to before r1420006. Then I > just added some deprecations. I should not have introduced new > constructors or RandomDataGenerator at all at this point.
Sorry. Misspoke there. What I meant was I should not have introduced a constructor using RandomDataGenerator. I *did* introduce one using RandomGenerator, which is what should be used. Sorry for the bad communication and somewhat convoluted naming on this. Hopefully this makes sense. Phil > I am > sorry I did a bad job explaining what was going on there. > Basically, EmpiricalDistibution needs a RandomDataGenerator / > RandomDataImpl to generate data following certain distributions. > What should be provided as a constructor argument for this class > (and ValueServer) is a RandomGenerator, which is the underlying > source of random data. The RandomDataImpl constructors are really > legacy, going back to the days before RandomGenerator existed. Does > this make sense? > > Phil >> >> Gilles >> >>> Fixed javadoc error in EmpiricalDistribution class javadoc. >>> Deprecated constructors taking RandomDataImpl instances in >>> EmpiricalDistribution, ValueServer. These constructors predate >>> RandomGenerator, which should be used directly as the source of random data >>> for these classes. >>> >>> Modified: >>> >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/EmpiricalDistribution.java >>> >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/ValueServer.java >>> >>> Modified: >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/EmpiricalDistribution.java >>> URL: >>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/EmpiricalDistribution.java?rev=1421968&r1=1421967&r2=1421968&view=diff >>> ============================================================================== >>> --- >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/EmpiricalDistribution.java >>> (original) >>> +++ >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/EmpiricalDistribution.java >>> Fri Dec 14 16:28:23 2012 >>> @@ -29,8 +29,8 @@ import java.util.ArrayList; >>> import java.util.List; >>> >>> import org.apache.commons.math3.distribution.AbstractRealDistribution; >>> -import org.apache.commons.math3.distribution.RealDistribution; >>> import org.apache.commons.math3.distribution.NormalDistribution; >>> +import org.apache.commons.math3.distribution.RealDistribution; >>> import org.apache.commons.math3.exception.MathIllegalStateException; >>> import org.apache.commons.math3.exception.MathInternalError; >>> import org.apache.commons.math3.exception.NullArgumentException; >>> @@ -134,18 +134,14 @@ public class EmpiricalDistribution exten >>> /** upper bounds of subintervals in (0,1) "belonging" to the bins */ >>> private double[] upperBounds = null; >>> >>> - /** Data generator. */ >>> - private final RandomDataGenerator randomDataGen; >>> - /** >>> - * XXX Enable backward-compatibility (to be removed in 4.0). >>> - */ >>> - private final boolean useRandomDataImpl; >>> + /** RandomDataImpl instance to use in repeated calls to getNext() */ >>> + private final RandomDataImpl randomData; >>> >>> /** >>> * Creates a new EmpiricalDistribution with the default bin count. >>> */ >>> public EmpiricalDistribution() { >>> - this(DEFAULT_BIN_COUNT); >>> + this(DEFAULT_BIN_COUNT, new RandomDataImpl()); >>> } >>> >>> /** >>> @@ -154,7 +150,7 @@ public class EmpiricalDistribution exten >>> * @param binCount number of bins >>> */ >>> public EmpiricalDistribution(int binCount) { >>> - this(binCount, (RandomGenerator) null); >>> + this(binCount, new RandomDataImpl()); >>> } >>> >>> /** >>> @@ -162,82 +158,52 @@ public class EmpiricalDistribution exten >>> * provided {@link RandomGenerator} as the source of random data. >>> * >>> * @param binCount number of bins >>> - * @param randomData random data generator (may be null, resulting in >>> a default generator) >>> - * @deprecated As of 3.1. To be removed in 4.0. Please use >>> - * {@link #EmpiricalDistribution(int,RandomDataGenerator)} instead. >>> + * @param generator random data generator (may be null, resulting in >>> default JDK generator) >>> + * @since 3.0 >>> */ >>> - @Deprecated >>> - public EmpiricalDistribution(int binCount, RandomDataImpl randomData) { >>> + public EmpiricalDistribution(int binCount, RandomGenerator generator) { >>> this.binCount = binCount; >>> - this.randomData = randomData == null ? >>> - new RandomDataImpl() : >>> - randomData; >>> + randomData = new RandomDataImpl(generator); >>> binStats = new ArrayList<SummaryStatistics>(); >>> - useRandomDataImpl = true; >>> - randomDataGen = null; >>> - } >>> - /** >>> - * Creates a new EmpiricalDistribution with the specified bin count >>> using the >>> - * provided {@link RandomGenerator} as the source of random data. >>> - * >>> - * @param randomData random data generator (may be null, resulting in >>> a default generator) >>> - * @deprecated As of 3.1. To be removed in 4.0. Please use >>> - * {@link #EmpiricalDistribution(RandomDataGenerator)} instead. >>> - */ >>> - @Deprecated >>> - public EmpiricalDistribution(RandomDataImpl randomData) { >>> - this(DEFAULT_BIN_COUNT, randomData); >>> } >>> >>> /** >>> - * Creates a new EmpiricalDistribution with the specified bin count >>> using the >>> - * provided {@link RandomGenerator} as the source of random data. >>> - * >>> - * @param binCount number of bins >>> - * @param randomData random data generator (may be null, resulting in >>> a default generator) >>> - */ >>> - public EmpiricalDistribution(int binCount, RandomDataGenerator >>> randomData) { >>> - this.binCount = binCount; >>> - this.randomDataGen = randomData == null ? >>> - new RandomDataGenerator() : >>> - randomData; >>> - binStats = new ArrayList<SummaryStatistics>(); >>> - useRandomDataImpl = false; // XXX Remove in 4.0 >>> - } >>> - /** >>> - * Creates a new EmpiricalDistribution with the specified bin count >>> using the >>> + * Creates a new EmpiricalDistribution with default bin count using the >>> * provided {@link RandomGenerator} as the source of random data. >>> * >>> - * @param randomData random data generator (may be null, resulting in >>> a default generator) >>> + * @param generator random data generator (may be null, resulting in >>> default JDK generator) >>> + * @since 3.0 >>> */ >>> - public EmpiricalDistribution(RandomDataGenerator randomData) { >>> - this(DEFAULT_BIN_COUNT, randomData); >>> + public EmpiricalDistribution(RandomGenerator generator) { >>> + this(DEFAULT_BIN_COUNT, generator); >>> } >>> >>> /** >>> * Creates a new EmpiricalDistribution with the specified bin count >>> using the >>> - * provided {@link RandomGenerator} as the source of random data. >>> + * provided {@link RandomDataImpl} instance as the source of random >>> data. >>> * >>> * @param binCount number of bins >>> - * @param generator random data generator (may be null, resulting in a >>> default generator) >>> + * @param randomData random data generator (may be null, resulting in >>> default JDK generator) >>> * @since 3.0 >>> */ >>> - public EmpiricalDistribution(int binCount, RandomGenerator generator) { >>> - this(binCount, new RandomDataGenerator(generator)); >>> + public EmpiricalDistribution(int binCount, RandomDataImpl randomData) { >>> + this.binCount = binCount; >>> + this.randomData = randomData; >>> + binStats = new ArrayList<SummaryStatistics>(); >>> } >>> >>> /** >>> * Creates a new EmpiricalDistribution with default bin count using the >>> - * provided {@link RandomGenerator} as the source of random data. >>> + * provided {@link RandomDataImpl} as the source of random data. >>> * >>> - * @param generator random data generator (may be null, resulting in >>> default generator) >>> + * @param randomData random data generator (may be null, resulting in >>> default JDK generator) >>> * @since 3.0 >>> */ >>> - public EmpiricalDistribution(RandomGenerator generator) { >>> - this(DEFAULT_BIN_COUNT, generator); >>> + public EmpiricalDistribution(RandomDataImpl randomData) { >>> + this(DEFAULT_BIN_COUNT, randomData); >>> } >>> >>> - /** >>> + /** >>> * Computes the empirical distribution from the provided >>> * array of numbers. >>> * >>> @@ -288,7 +254,7 @@ public class EmpiricalDistribution exten >>> } finally { >>> try { >>> in.close(); >>> - } catch (IOException ex) { // NOPMD >>> + } catch (IOException ex) { >>> // ignore >>> } >>> } >>> @@ -320,7 +286,7 @@ public class EmpiricalDistribution exten >>> } finally { >>> try { >>> in.close(); >>> - } catch (IOException ex) { // NOPMD >>> + } catch (IOException ex) { >>> // ignore >>> } >>> } >>> @@ -497,41 +463,22 @@ public class EmpiricalDistribution exten >>> throw new >>> MathIllegalStateException(LocalizedFormats.DISTRIBUTION_NOT_LOADED); >>> } >>> >>> - if (useRandomDataImpl) { >>> - // XXX backward compatibility. >>> - // Start with a uniformly distributed random number in (0, 1) >>> - final double x = randomData.nextUniform(0,1); >>> - // Use this to select the bin and generate a Gaussian within >>> the bin >>> - for (int i = 0; i < binCount; i++) { >>> - if (x <= upperBounds[i]) { >>> - SummaryStatistics stats = binStats.get(i); >>> - if (stats.getN() > 0) { >>> - if (stats.getStandardDeviation() > 0) { // more >>> than one obs >>> - return randomData.nextGaussian(stats.getMean(), >>> - >>> stats.getStandardDeviation()); >>> - } else { >>> - return stats.getMean(); // only one obs in bin >>> - } >>> - } >>> - } >>> - } >>> - } else { >>> - // Start with a uniformly distributed random number in (0, 1) >>> - final double x = randomDataGen.nextUniform(0, 1); >>> - // Use this to select the bin and generate a Gaussian within >>> the bin >>> - for (int i = 0; i < binCount; i++) { >>> - if (x <= upperBounds[i]) { >>> - SummaryStatistics stats = binStats.get(i); >>> - if (stats.getN() > 0) { >>> - if (stats.getStandardDeviation() > 0) { // more >>> than one obs >>> - return >>> randomDataGen.nextGaussian(stats.getMean(), >>> - >>> stats.getStandardDeviation()); >>> - } else { >>> - return stats.getMean(); // only one obs in bin >>> - } >>> - } >>> - } >>> - } >>> + // Start with a uniformly distributed random number in (0,1) >>> + final double x = randomData.nextUniform(0,1); >>> + >>> + // Use this to select the bin and generate a Gaussian within the >>> bin >>> + for (int i = 0; i < binCount; i++) { >>> + if (x <= upperBounds[i]) { >>> + SummaryStatistics stats = binStats.get(i); >>> + if (stats.getN() > 0) { >>> + if (stats.getStandardDeviation() > 0) { // more than >>> one obs >>> + return randomData.nextGaussian(stats.getMean(), >>> + >>> stats.getStandardDeviation()); >>> + } else { >>> + return stats.getMean(); // only one obs in bin >>> + } >>> + } >>> + } >>> } >>> throw new >>> MathIllegalStateException(LocalizedFormats.NO_BIN_SELECTED); >>> } >>> @@ -624,12 +571,7 @@ public class EmpiricalDistribution exten >>> * @since 3.0 >>> */ >>> public void reSeed(long seed) { >>> - if (useRandomDataImpl) { >>> - // XXX backward compatibility. >>> - randomData.reSeed(seed); >>> - } else { >>> - randomDataGen.reSeed(seed); >>> - } >>> + randomData.reSeed(seed); >>> } >>> >>> // Distribution methods --------------------------- >>> @@ -819,7 +761,7 @@ public class EmpiricalDistribution exten >>> */ >>> @Override >>> public void reseedRandomGenerator(long seed) { >>> - reSeed(seed); >>> + randomData.reSeed(seed); >>> } >>> >>> /** >>> >>> Modified: >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/ValueServer.java >>> URL: >>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/ValueServer.java?rev=1421968&r1=1421967&r2=1421968&view=diff >>> ============================================================================== >>> --- >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/ValueServer.java >>> (original) >>> +++ >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/ValueServer.java >>> Fri Dec 14 16:28:23 2012 >>> @@ -88,35 +88,36 @@ public class ValueServer { >>> private BufferedReader filePointer = null; >>> >>> /** RandomDataImpl to use for random data generation. */ >>> - private final RandomDataGenerator randomData; >>> + private final RandomDataImpl randomData; >>> >>> // Data generation modes ====================================== >>> >>> /** Creates new ValueServer */ >>> public ValueServer() { >>> - randomData = new RandomDataGenerator(); >>> + randomData = new RandomDataImpl(); >>> } >>> >>> /** >>> - * Construct a ValueServer instance using a RandomDataGenerator as its >>> source >>> + * Construct a ValueServer instance using a RandomDataImpl as its >>> source >>> * of random data. >>> * >>> - * @param randomData random data source >>> + * @param randomData the RandomDataImpl instance used to source random >>> data >>> * @since 3.0 >>> + * @deprecated use {@link #ValueServer(RandomGenerator)} >>> */ >>> - public ValueServer(RandomDataGenerator randomData) { >>> + public ValueServer(RandomDataImpl randomData) { >>> this.randomData = randomData; >>> } >>> + >>> /** >>> - * Construct a ValueServer instance using a RandomDataImpl as its >>> source >>> + * Construct a ValueServer instance using a RandomGenerator as its >>> source >>> * of random data. >>> * >>> - * @param randomData random data source >>> - * @deprecated As of 3.1. Use {@link >>> #ValueServer(RandomDataGenerator)} instead. >>> + * @since 3.1 >>> + * @param generator source of random data >>> */ >>> - @Deprecated >>> - public ValueServer(RandomDataImpl randomData) { >>> - this(randomData.getDelegate()); >>> + public ValueServer(RandomGenerator generator) { >>> + this.randomData = new RandomDataImpl(generator); >>> } >>> >>> /** >>> @@ -288,7 +289,7 @@ public class ValueServer { >>> try { >>> filePointer.close(); >>> filePointer = null; >>> - } catch (IOException ex) { // NOPMD >>> + } catch (IOException ex) { >>> // ignore >>> } >>> } >>> >>> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org