On 12/14/12 11:59 AM, Gilles Sadowski wrote: > 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)?
Right, there will be no *constructor* taking one of those. There will be a member variable, constructed from a RandomGenerator, which will change from being a RandomDataImpl to a RandomDataGenerator. We could make that change now if RandomDataImpl exposed a getDelegate method to deliver the wrapped RandomDataGenerator. Then the current member variable could be changed to a RandomDataGenerator and the (now deprecated) constructor taking a RandomDataImpl could convert it. That is a little more work to implement and probably not worth the hassle for 3.1 as the member field is private. Phil > > 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 > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
