commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t.@apache.org
Subject [1/2] [math] [MATH-958] Remove support for NaNStrategy#REMOVED in SpearmansCorrelation.
Date Mon, 02 Mar 2015 21:41:56 GMT
Repository: commons-math
Updated Branches:
  refs/heads/master 0a5cd1132 -> 1e7d4f057


[MATH-958] Remove support for NaNStrategy#REMOVED in SpearmansCorrelation.


Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/4e08e17e
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/4e08e17e
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/4e08e17e

Branch: refs/heads/master
Commit: 4e08e17ec536038c9c0dd888f77b912cae82816c
Parents: 0a5cd11
Author: Thomas Neidhart <thomas.neidhart@gmail.com>
Authored: Mon Mar 2 22:41:18 2015 +0100
Committer: Thomas Neidhart <thomas.neidhart@gmail.com>
Committed: Mon Mar 2 22:41:18 2015 +0100

----------------------------------------------------------------------
 src/changes/changes.xml                         |   4 +
 .../math4/exception/util/LocalizedFormats.java  |   1 +
 .../stat/correlation/SpearmansCorrelation.java  | 109 +++++--------------
 .../util/LocalizedFormats_fr.properties         |   1 +
 .../exception/util/LocalizedFormatsTest.java    |   2 +-
 .../SpearmansRankCorrelationTest.java           |   9 +-
 6 files changed, 38 insertions(+), 88 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/commons-math/blob/4e08e17e/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index ee73fca..4e5eb4d 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -55,6 +55,10 @@ If the output is not quite correct, check for invisible trailing spaces!
 
     <release version="4.0" date="XXXX-XX-XX" description="">
       <action dev="tn" type="update" issue="MATH-869">
+        "SpearmansCorrelation" will now throw an "MathIllegalArgumentException"
+        if provided with a "NaturalRanking" instance that uses "REMOVED" as "NaNStrategy".
+      </action>
+      <action dev="tn" type="update" issue="MATH-869">
         "NullArgumentException" extends now "java.lang.NullPointerException"
         instead of "MathIllegalArgumentException".
       </action>

http://git-wip-us.apache.org/repos/asf/commons-math/blob/4e08e17e/src/main/java/org/apache/commons/math4/exception/util/LocalizedFormats.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/exception/util/LocalizedFormats.java b/src/main/java/org/apache/commons/math4/exception/util/LocalizedFormats.java
index 373bc19..6d300ab 100644
--- a/src/main/java/org/apache/commons/math4/exception/util/LocalizedFormats.java
+++ b/src/main/java/org/apache/commons/math4/exception/util/LocalizedFormats.java
@@ -241,6 +241,7 @@ public enum LocalizedFormats implements Localizable {
     NOT_STRICTLY_INCREASING_SEQUENCE("points {3} and {2} are not strictly increasing ({1}
>= {0})"), /* keep */
     NOT_SUBTRACTION_COMPATIBLE_MATRICES("{0}x{1} and {2}x{3} matrices are not subtraction
compatible"),
     NOT_SUPPORTED_IN_DIMENSION_N("method not supported in dimension {0}"),
+    NOT_SUPPORTED_NAN_STRATEGY("NaN strategy {0} not supported"),
     NOT_SYMMETRIC_MATRIX("not symmetric matrix"),
     NON_SYMMETRIC_MATRIX("non symmetric matrix: the difference between entries at ({0},{1})
and ({1},{0}) is larger than {2}"), /* keep */
     NO_BIN_SELECTED("no bin selected"),

http://git-wip-us.apache.org/repos/asf/commons-math/blob/4e08e17e/src/main/java/org/apache/commons/math4/stat/correlation/SpearmansCorrelation.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/stat/correlation/SpearmansCorrelation.java
b/src/main/java/org/apache/commons/math4/stat/correlation/SpearmansCorrelation.java
index 1afc940..0e436ff 100644
--- a/src/main/java/org/apache/commons/math4/stat/correlation/SpearmansCorrelation.java
+++ b/src/main/java/org/apache/commons/math4/stat/correlation/SpearmansCorrelation.java
@@ -17,11 +17,6 @@
 
 package org.apache.commons.math4.stat.correlation;
 
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
 import org.apache.commons.math4.exception.DimensionMismatchException;
 import org.apache.commons.math4.exception.MathIllegalArgumentException;
 import org.apache.commons.math4.exception.util.LocalizedFormats;
@@ -62,14 +57,21 @@ public class SpearmansCorrelation {
 
     /**
      * Create a SpearmansCorrelation with the given ranking algorithm.
-     * <p>
-     * From version 4.0 onwards this constructor will throw an exception
-     * if the provided {@link NaturalRanking} uses a {@link NaNStrategy#REMOVED} strategy.
      *
      * @param rankingAlgorithm ranking algorithm
+     * @throws MathIllegalArgumentException if the provided {@link RankingAlgorithm} is of
+     * type {@link NaturalRanking} and uses a {@link NaNStrategy#REMOVED} strategy
      * @since 3.1
      */
-    public SpearmansCorrelation(final RankingAlgorithm rankingAlgorithm) {
+    public SpearmansCorrelation(final RankingAlgorithm rankingAlgorithm)
+        throws MathIllegalArgumentException {
+
+        if (rankingAlgorithm instanceof NaturalRanking &&
+            NaNStrategy.REMOVED == ((NaturalRanking) rankingAlgorithm).getNanStrategy())
{
+            throw new MathIllegalArgumentException(LocalizedFormats.NOT_SUPPORTED_NAN_STRATEGY,
+                                                   NaNStrategy.REMOVED);
+        }
+
         data = null;
         this.rankingAlgorithm = rankingAlgorithm;
         rankCorrelation = null;
@@ -88,15 +90,22 @@ public class SpearmansCorrelation {
     /**
      * Create a SpearmansCorrelation with the given input data matrix
      * and ranking algorithm.
-     * <p>
-     * From version 4.0 onwards this constructor will throw an exception
-     * if the provided {@link NaturalRanking} uses a {@link NaNStrategy#REMOVED} strategy.
      *
      * @param dataMatrix matrix of data with columns representing
      * variables to correlate
      * @param rankingAlgorithm ranking algorithm
+     * @throws MathIllegalArgumentException if the provided {@link RankingAlgorithm} is of
+     * type {@link NaturalRanking} and uses a {@link NaNStrategy#REMOVED} strategy
      */
-    public SpearmansCorrelation(final RealMatrix dataMatrix, final RankingAlgorithm rankingAlgorithm)
{
+    public SpearmansCorrelation(final RealMatrix dataMatrix, final RankingAlgorithm rankingAlgorithm)
+        throws MathIllegalArgumentException {
+
+        if (rankingAlgorithm instanceof NaturalRanking &&
+            NaNStrategy.REMOVED == ((NaturalRanking) rankingAlgorithm).getNanStrategy())
{
+            throw new MathIllegalArgumentException(LocalizedFormats.NOT_SUPPORTED_NAN_STRATEGY,
+                                                   NaNStrategy.REMOVED);
+        }
+
         this.rankingAlgorithm = rankingAlgorithm;
         this.data = rankTransform(dataMatrix);
         rankCorrelation = new PearsonsCorrelation(data);
@@ -167,19 +176,8 @@ public class SpearmansCorrelation {
             throw new MathIllegalArgumentException(LocalizedFormats.INSUFFICIENT_DIMENSION,
                                                    xArray.length, 2);
         } else {
-            double[] x = xArray;
-            double[] y = yArray;
-            if (rankingAlgorithm instanceof NaturalRanking &&
-                NaNStrategy.REMOVED == ((NaturalRanking) rankingAlgorithm).getNanStrategy())
{
-                final Set<Integer> nanPositions = new HashSet<Integer>();
-
-                nanPositions.addAll(getNaNPositions(xArray));
-                nanPositions.addAll(getNaNPositions(yArray));
-
-                x = removeValues(xArray, nanPositions);
-                y = removeValues(yArray, nanPositions);
-            }
-            return new PearsonsCorrelation().correlation(rankingAlgorithm.rank(x), rankingAlgorithm.rank(y));
+            return new PearsonsCorrelation().correlation(rankingAlgorithm.rank(xArray),
+                                                         rankingAlgorithm.rank(yArray));
         }
     }
 
@@ -191,29 +189,7 @@ public class SpearmansCorrelation {
      * @return a rank-transformed matrix
      */
     private RealMatrix rankTransform(final RealMatrix matrix) {
-        RealMatrix transformed = null;
-
-        if (rankingAlgorithm instanceof NaturalRanking &&
-                ((NaturalRanking) rankingAlgorithm).getNanStrategy() == NaNStrategy.REMOVED)
{
-            final Set<Integer> nanPositions = new HashSet<Integer>();
-            for (int i = 0; i < matrix.getColumnDimension(); i++) {
-                nanPositions.addAll(getNaNPositions(matrix.getColumn(i)));
-            }
-
-            // if we have found NaN values, we have to update the matrix size
-            if (!nanPositions.isEmpty()) {
-                transformed = new BlockRealMatrix(matrix.getRowDimension() - nanPositions.size(),
-                                                  matrix.getColumnDimension());
-                for (int i = 0; i < transformed.getColumnDimension(); i++) {
-                    transformed.setColumn(i, removeValues(matrix.getColumn(i), nanPositions));
-                }
-            }
-        }
-
-        if (transformed == null) {
-            transformed = matrix.copy();
-        }
-
+        RealMatrix transformed = matrix.copy();
         for (int i = 0; i < transformed.getColumnDimension(); i++) {
             transformed.setColumn(i, rankingAlgorithm.rank(transformed.getColumn(i)));
         }
@@ -221,39 +197,4 @@ public class SpearmansCorrelation {
         return transformed;
     }
 
-    /**
-     * Returns a list containing the indices of NaN values in the input array.
-     *
-     * @param input the input array
-     * @return a list of NaN positions in the input array
-     */
-    private List<Integer> getNaNPositions(final double[] input) {
-        final List<Integer> positions = new ArrayList<Integer>();
-        for (int i = 0; i < input.length; i++) {
-            if (Double.isNaN(input[i])) {
-                positions.add(i);
-            }
-        }
-        return positions;
-    }
-
-    /**
-     * Removes all values from the input array at the specified indices.
-     *
-     * @param input the input array
-     * @param indices a set containing the indices to be removed
-     * @return the input array without the values at the specified indices
-     */
-    private double[] removeValues(final double[] input, final Set<Integer> indices)
{
-        if (indices.isEmpty()) {
-            return input;
-        }
-        final double[] result = new double[input.length - indices.size()];
-        for (int i = 0, j = 0; i < input.length; i++) {
-            if (!indices.contains(i)) {
-                result[j++] = input[i];
-            }
-        }
-        return result;
-    }
 }

http://git-wip-us.apache.org/repos/asf/commons-math/blob/4e08e17e/src/main/resources/assets/org/apache/commons/math4/exception/util/LocalizedFormats_fr.properties
----------------------------------------------------------------------
diff --git a/src/main/resources/assets/org/apache/commons/math4/exception/util/LocalizedFormats_fr.properties
b/src/main/resources/assets/org/apache/commons/math4/exception/util/LocalizedFormats_fr.properties
index dc9691c..678546a 100644
--- a/src/main/resources/assets/org/apache/commons/math4/exception/util/LocalizedFormats_fr.properties
+++ b/src/main/resources/assets/org/apache/commons/math4/exception/util/LocalizedFormats_fr.properties
@@ -214,6 +214,7 @@ NOT_STRICTLY_INCREASING_NUMBER_OF_POINTS = les points {0} et {1} ne sont
pas str
 NOT_STRICTLY_INCREASING_SEQUENCE = les points {3} et {2} ne sont pas strictement croissants
({1} >= {0})
 NOT_SUBTRACTION_COMPATIBLE_MATRICES = les dimensions {0}x{1} et {2}x{3} sont incompatibles
pour la soustraction matricielle
 NOT_SUPPORTED_IN_DIMENSION_N = m\u00e9thode non disponible en dimension {0}
+NOT_SUPPORTED_NAN_STRATEGY = strat\u00e9gie "NaN" {0} non disponible
 NOT_SYMMETRIC_MATRIX = matrice non symm\u00e9trique
 NON_SYMMETRIC_MATRIX = matrice non symm\u00e9trique: la diff\u00e9rence entre les \u00e9l\u00e9ments
({0},{1}) et ({1},{0}) est sup\u00e9rieure \u00e0 {2}
 NO_BIN_SELECTED = aucun compartiment s\u00e9lectionn\u00e9

http://git-wip-us.apache.org/repos/asf/commons-math/blob/4e08e17e/src/test/java/org/apache/commons/math4/exception/util/LocalizedFormatsTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/exception/util/LocalizedFormatsTest.java
b/src/test/java/org/apache/commons/math4/exception/util/LocalizedFormatsTest.java
index a348757..b497780 100644
--- a/src/test/java/org/apache/commons/math4/exception/util/LocalizedFormatsTest.java
+++ b/src/test/java/org/apache/commons/math4/exception/util/LocalizedFormatsTest.java
@@ -29,7 +29,7 @@ public class LocalizedFormatsTest {
 
     @Test
     public void testMessageNumber() {
-        Assert.assertEquals(321, LocalizedFormats.values().length);
+        Assert.assertEquals(322, LocalizedFormats.values().length);
     }
 
     @Test

http://git-wip-us.apache.org/repos/asf/commons-math/blob/4e08e17e/src/test/java/org/apache/commons/math4/stat/correlation/SpearmansRankCorrelationTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/math4/stat/correlation/SpearmansRankCorrelationTest.java
b/src/test/java/org/apache/commons/math4/stat/correlation/SpearmansRankCorrelationTest.java
index 59cf485..fbccc29 100644
--- a/src/test/java/org/apache/commons/math4/stat/correlation/SpearmansRankCorrelationTest.java
+++ b/src/test/java/org/apache/commons/math4/stat/correlation/SpearmansRankCorrelationTest.java
@@ -17,6 +17,7 @@
 package org.apache.commons.math4.stat.correlation;
 
 import org.apache.commons.math4.TestUtils;
+import org.apache.commons.math4.exception.MathIllegalArgumentException;
 import org.apache.commons.math4.linear.BlockRealMatrix;
 import org.apache.commons.math4.linear.MatrixUtils;
 import org.apache.commons.math4.linear.RealMatrix;
@@ -121,19 +122,21 @@ public class SpearmansRankCorrelationTest extends PearsonsCorrelationTest
{
                 new SpearmansCorrelation().computeCorrelationMatrix(data), Double.MIN_VALUE);
     }
 
-    @Test
+    @Test(expected = MathIllegalArgumentException.class)
     public void testMath891Array() {
+        // NaNStrategy.REMOVED is not supported since 4.0
         final double[] xArray = new double[] { Double.NaN, 1.9, 2, 100, 3 };
         final double[] yArray = new double[] { 10, 2, 10, Double.NaN, 4 };
 
         NaturalRanking ranking = new NaturalRanking(NaNStrategy.REMOVED);
         SpearmansCorrelation spearman = new SpearmansCorrelation(ranking);
-        
+
         Assert.assertEquals(0.5, spearman.correlation(xArray, yArray), Double.MIN_VALUE);
     }
 
-    @Test
+    @Test(expected = MathIllegalArgumentException.class)
     public void testMath891Matrix() {
+        // NaNStrategy.REMOVED is not supported since 4.0
         final double[] xArray = new double[] { Double.NaN, 1.9, 2, 100, 3 };
         final double[] yArray = new double[] { 10, 2, 10, Double.NaN, 4 };
 


Mime
View raw message