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 <[email protected]> Authored: Mon Mar 2 22:41:18 2015 +0100 Committer: Thomas Neidhart <[email protected]> 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 };
