You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ps...@apache.org on 2012/12/14 17:28:25 UTC

svn commit: r1421968 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random: EmpiricalDistribution.java ValueServer.java

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.
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
             }
         }



Re: svn commit: r1421968 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random: EmpiricalDistribution.java ValueServer.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Fri, Dec 14, 2012 at 01:29:44PM -0800, Phil Steitz wrote:
> On 12/14/12 12:58 PM, Gilles Sadowski wrote:
> > On Fri, Dec 14, 2012 at 12:14:45PM -0800, Phil Steitz wrote:
> >> 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, 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)?
> >> 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. 
> > I don't understand why "EmpiricalDistribution" would need a
> > "RandomDataGenerator" at all. That duplicates the "random source" readily
> > available from the parent class. 
> 
> RandomDataGenerator extends the capabilities of RandomGenerator and
> EmpiricalDistribution uses the extended capabilities.
> 
> > [That was part of my comment to MATH-915.]
> > The "randomData" field is only actually used in "getNextValue" where the
> > code could directly use "UniformRealDistribution" and "NormalDistribution"
> > variables instead (as my overwritten fix was doing).
> 
> Your version was using RandomDataGenerator / RandomDataImpl
> instances to do the work.  In theory, you could use distribution
> instances to do this; but this is precisely the kind of use case
> that RandomDataImpl was created to address - direct access to data
> generation methods following common parametric distributions.
> 
> 
> >
> >> We could make that change now if RandomDataImpl exposed a
> >> getDelegate method to deliver the wrapped RandomDataGenerator.  
> > It does. That was one of my changes to fix MATH-916.
> 
> Great.  We think alike (sometimes :)
> 
> Given that, we could s/RandomDataImpl/RandomDataGenerator in the
> private field.

I'll commit that in a few minutes...

Gilles

> 
> Phil
> >
> > Gilles
> >
> >> 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-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
> >>>
> >>>
> >>
> >> ---------------------------------------------------------------------
> >> 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
> 

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


Re: svn commit: r1421968 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random: EmpiricalDistribution.java ValueServer.java

Posted by Phil Steitz <ph...@gmail.com>.
On 12/14/12 12:58 PM, Gilles Sadowski wrote:
> On Fri, Dec 14, 2012 at 12:14:45PM -0800, Phil Steitz wrote:
>> 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, 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)?
>> 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. 
> I don't understand why "EmpiricalDistribution" would need a
> "RandomDataGenerator" at all. That duplicates the "random source" readily
> available from the parent class. 

RandomDataGenerator extends the capabilities of RandomGenerator and
EmpiricalDistribution uses the extended capabilities.

> [That was part of my comment to MATH-915.]
> The "randomData" field is only actually used in "getNextValue" where the
> code could directly use "UniformRealDistribution" and "NormalDistribution"
> variables instead (as my overwritten fix was doing).

Your version was using RandomDataGenerator / RandomDataImpl
instances to do the work.  In theory, you could use distribution
instances to do this; but this is precisely the kind of use case
that RandomDataImpl was created to address - direct access to data
generation methods following common parametric distributions.


>
>> We could make that change now if RandomDataImpl exposed a
>> getDelegate method to deliver the wrapped RandomDataGenerator.  
> It does. That was one of my changes to fix MATH-916.

Great.  We think alike (sometimes :)

Given that, we could s/RandomDataImpl/RandomDataGenerator in the
private field.

Phil
>
> Gilles
>
>> 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-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
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> 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


Re: svn commit: r1421968 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random: EmpiricalDistribution.java ValueServer.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Fri, Dec 14, 2012 at 12:14:45PM -0800, Phil Steitz wrote:
> 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, 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)?
> 
> 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. 

I don't understand why "EmpiricalDistribution" would need a
"RandomDataGenerator" at all. That duplicates the "random source" readily
available from the parent class. [That was part of my comment to MATH-915.]
The "randomData" field is only actually used in "getNextValue" where the
code could directly use "UniformRealDistribution" and "NormalDistribution"
variables instead (as my overwritten fix was doing).

> We could make that change now if RandomDataImpl exposed a
> getDelegate method to deliver the wrapped RandomDataGenerator.  

It does. That was one of my changes to fix MATH-916.

Gilles

> 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-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
> >
> >
> 
> 
> ---------------------------------------------------------------------
> 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


Re: svn commit: r1421968 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random: EmpiricalDistribution.java ValueServer.java

Posted by Phil Steitz <ph...@gmail.com>.
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, 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)?

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-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
>
>


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


Re: svn commit: r1421968 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random: EmpiricalDistribution.java ValueServer.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
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


Re: svn commit: r1421968 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random: EmpiricalDistribution.java ValueServer.java

Posted by Phil Steitz <ph...@gmail.com>.
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.

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


Re: svn commit: r1421968 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random: EmpiricalDistribution.java ValueServer.java

Posted by Phil Steitz <ph...@gmail.com>.
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


Re: svn commit: r1421968 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random: EmpiricalDistribution.java ValueServer.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
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).


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