On Tue, 13 Aug 2013 10:52:10 -0700, Phil Steitz wrote:
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
Please modify as you see fit.
[IMHO, the previous was not so clear. Also, I agree that the Javadoc
should indicate
code does, but I don't think that it should be required to prove that
what it does
yields the expected result (that's for the references to provide).
Rather, the unit
tests would ideally spot that the algorithm is wrong or buggy.]
Gilles
*/
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org