commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: svn commit: r1421968 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random: EmpiricalDistribution.java ValueServer.java
Date Fri, 14 Dec 2012 20:14:45 GMT
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


Mime
View raw message