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: r1513501 - in /commons/proper/math/trunk: LICENSE.txt src/main/java/org/apache/commons/math3/random/RandomDataGenerator.java src/main/java/org/apache/commons/math3/util/MathArrays.java
Date Tue, 13 Aug 2013 17:52:10 GMT
On 8/13/13 10:27 AM, Phil Steitz wrote:
> On 8/13/13 7:20 AM, erans@apache.org wrote:
>> Author: erans
>> Date: Tue Aug 13 14:20:26 2013
>> New Revision: 1513501
>>
>> URL: http://svn.apache.org/r1513501
>> Log:
>> MATH-1019
>> Removed dead link in Javadoc; added entry to original reference
>> in "LICENCE" file.
>>
>> Modified:
>>     commons/proper/math/trunk/LICENSE.txt
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/RandomDataGenerator.java
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/MathArrays.java
>>
>> Modified: commons/proper/math/trunk/LICENSE.txt
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/LICENSE.txt?rev=1513501&r1=1513500&r2=1513501&view=diff
>> ==============================================================================
>> --- commons/proper/math/trunk/LICENSE.txt (original)
>> +++ commons/proper/math/trunk/LICENSE.txt Tue Aug 13 14:20:26 2013
>> @@ -385,3 +385,16 @@ Th Orekit library is described at:
>>    https://www.orekit.org/forge/projects/orekit
>>  The original files are distributed under the terms of the Apache 2 license
>>  which is: Copyright 2010 CS Communication & Systèmes
>> +
>> +===============================================================================
>> +
>> +The initial code for shuffling an array (originally in class
>> +"org.apache.commons.math3.random.RandomDataGenerator", now replaced by
>> +a method in class "org.apache.commons.math3.util.MathArrays") was
>> +inspired from the algorithm description provided in
>> +"Algorithms", by Ian Craw and John Pulham (University of Aberdeen 1999).
>> +The textbook (containing a proof that the shuffle is uniformly random) is
>> +available here:
>> +  http://citeseerx.ist.psu.edu/viewdoc/download;?doi=10.1.1.173.1898&rep=rep1&type=pdf
>> +
>> +===============================================================================
> No need to add to LICENSE and probably wrong to do so here.  There
> was no code consulted or used in the reference.  Just the
> algorithm.  We should not fill up LICENSE with references to sources
> for *algorithms* - only previously licensed code used directly or
> used as a basis for implementation.   Best to drop from here and
> (optionally) add the reference to the javadoc.  The value of adding
> it there is that it provides background and justification for the
> algorithm, much as other algorithm references elsewhere in the
> codebase do.
>
> Phil
>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/RandomDataGenerator.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/RandomDataGenerator.java?rev=1513501&r1=1513500&r2=1513501&view=diff
>> ==============================================================================
>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/RandomDataGenerator.java
(original)
>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/RandomDataGenerator.java
Tue Aug 13 14:20:26 2013
>> @@ -620,11 +620,10 @@ public class RandomDataGenerator impleme
>>      /**
>>       * {@inheritDoc}
>>       *
>> -     * <p>
>> -     * Uses a 2-cycle permutation shuffle. The shuffling process is described <a
>> -     * href="http://www.maths.abdn.ac.uk/~igc/tch/mx4002/notes/node83.html">
>> -     * here</a>.
>> -     * </p>
>> +     * This method calls {@link MathArrays#shuffle(int[],RandomGenerator)
>> +     * MathArrays.shuffle} in order to create a random shuffle of the set
>> +     * of natural numbers {@code { 0, 1, ..., n - 1 }}.
>> +     *
>>       * @throws NumberIsTooLargeException if {@code k > n}.
>>       * @throws NotStrictlyPositiveException if {@code k <= 0}.
>>       */
>> @@ -649,15 +648,8 @@ public class RandomDataGenerator impleme
>>      /**
>>       * {@inheritDoc}
>>       *
>> -     * <p>
>> -     * <strong>Algorithm Description</strong>: Uses a 2-cycle permutation
>> -     * shuffle to generate a random permutation of <code>c.size()</code>
and
>> -     * then returns the elements whose indexes correspond to the elements of the
>> -     * generated permutation. This technique is described, and proven to
>> -     * generate random samples <a
>> -     * href="http://www.maths.abdn.ac.uk/~igc/tch/mx4002/notes/node83.html">
>> -     * here</a>
>> -     * </p>
>> +     * This method calls {@link #nextPermutation(int,int) nextPermutation(c.size(),
k)}
>> +     * in order to sample the collection.

Sorry, missed above on first review.  I like the previous better, as
it describes how the sampling is actually done.  Just saying it
calls "nextPermutation" does not explain why or how it works.  I
would prefer to leave the Algorithm Description in there - making it
clear that what the code does is generate a random permutation and
then return elements at the first k indexes.

Phil
>>       */
>>      public Object[] nextSample(Collection<?> c, int k) throws NumberIsTooLargeException,
NotStrictlyPositiveException {
>>  
>>
>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/MathArrays.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/MathArrays.java?rev=1513501&r1=1513500&r2=1513501&view=diff
>> ==============================================================================
>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/MathArrays.java
(original)
>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/MathArrays.java
Tue Aug 13 14:20:26 2013
>> @@ -1442,6 +1442,8 @@ public class MathArrays {
>>       * The {@code start} and {@code pos} parameters select which portion
>>       * of the array is randomized and which is left untouched.
>>       *
>> +     * @see #shuffle(int[],int,Position,RandomGenerator)
>> +     *
>>       * @param list Array whose entries will be shuffled (in-place).
>>       * @param start Index at which shuffling begins.
>>       * @param pos Shuffling is performed for index positions between
>> @@ -1455,7 +1457,9 @@ public class MathArrays {
>>      }
>>  
>>      /**
>> -     * Shuffle the entries of the given array.
>> +     * Shuffle the entries of the given array, using the
>> +     * <a href="http://en.wikipedia.org/wiki/Fisher–Yates_shuffle#The_modern_algorithm">
>> +     * Fisher–Yates</a> algorithm.
>>       * The {@code start} and {@code pos} parameters select which portion
>>       * of the array is randomized and which is left untouched.
>>       *
>> @@ -1509,6 +1513,8 @@ public class MathArrays {
>>      /**
>>       * Shuffle the entries of the given array.
>>       *
>> +     * @see #shuffle(int[],int,Position,RandomGenerator)
>> +     *
>>       * @param list Array whose entries will be shuffled (in-place).
>>       * @param rng Random number generator.
>>       */
>> @@ -1520,6 +1526,8 @@ public class MathArrays {
>>      /**
>>       * Shuffle the entries of the given array.
>>       *
>> +     * @see #shuffle(int[],int,Position,RandomGenerator)
>> +     *
>>       * @param list Array whose entries will be shuffled (in-place).
>>       */
>>      public static void shuffle(int[] list) {
>>
>>
>>


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


Mime
View raw message