On Fri, Dec 14, 2012 at 10:03:14AM -0800, Phil Steitz wrote: > 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.
Si, ultimately, there will be neither a "RandomDataImpl" nor a "RandomDataGenerator" (as I suggested in the comments to MATH-915)? Gilles > > 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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
