On 8/13/13 10:27 AM, Phil Steitz wrote: > On 8/13/13 7:20 AM, er...@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-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org