commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: svn commit: r1421968 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random: EmpiricalDistribution.java ValueServer.java
Date Fri, 14 Dec 2012 17:43:30 GMT
On 12/14/12 8:52 AM, Gilles Sadowski wrote:
> On Fri, Dec 14, 2012 at 04:28:25PM -0000, psteitz@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.  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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message