commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
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 19:59:39 GMT
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, 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. 
> 
> 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-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
> 

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


Mime
View raw message