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

Reply via email to