Merge pull request #91 from cicirello/development

Various code improvements and minor optimizations

Author: cicirello

CHANGELOG.md

@@ -4,19 +4,26 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [Unreleased] - 2021-02-05
+## [Unreleased] - 2021-02-13
 
 ### Added
 
 ### Changed
+* Minor optimizations to ReversalDistance and ReinsertionDistance.
+* Refactored the RandomVariates.nextCauchy methods to remove redundancy.
+* Refactored org.cicirello.math.rand.BTPE to remove redundancy.
+* Refactored various methods of RandomIndexer to remove redundancy.
+* Minor optimizations to RandomIndexer.sample methods.
 
 ### Deprecated
 
 ### Removed
 
 ### Fixed
+* Fixed large magnitude negative input case in MathFunctions.logGamma.
 
 ### CI/CD
+* Test cases added to improve test coverage.
 
 
 ## [2.3.0] - 2021-01-30

src/org/cicirello/math/MathFunctions.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2019 Vincent A. Cicirello, <https://www.cicirello.org/>.
+ * Copyright 2019-2021 Vincent A. Cicirello, <https://www.cicirello.org/>.
  *
  * This file is part of JavaPermutationTools (https://jpt.cicirello.org/).
  *
@@ -26,8 +26,7 @@
  * MathFunctions is a class of utility methods that implement various mathematical functions.
  *
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a> 
- * @version 8.22.2019
- * @since 2.0
+ * @version 2.12.2021
  *
  */
 public final class MathFunctions {
@@ -75,16 +74,23 @@ public static double logGamma(double n) {
 			// input parameter is too large to calculate function
 			return Double.NEGATIVE_INFINITY;
 		} else if (n < -34.0) {
+			if (n < -4.5035996273704955e15) {
+				// inputs lower than this are essentially negative integers
+				// due to precision of floating-point numbers.
+				// This is the smallest negative double with an ulp less than 1.
+				return Double.POSITIVE_INFINITY;
+			}
 			n = -n; 
-			double p = ((int)n);
+			double p = ((long)n);
 			if (p == n) return Double.POSITIVE_INFINITY;
 			double z = n - p;
 			if (z > 0.5) {
 				p = p + 1.0;
 				z = p - n;
 			}
 			z = n * Math.sin(z * Math.PI);
-			if (z == 0.0) return Double.POSITIVE_INFINITY;
+			// This check doesn't seem to be necessary given addition of first check in block.
+			// if (z == 0.0) return Double.POSITIVE_INFINITY;
 			// ln(PI)
 			final double LOG_PI = 1.14472988584940017414;
 			return LOG_PI - Math.log(z) - logGamma(n);

src/org/cicirello/math/la/JacobiDiagonalization.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2018 Vincent A. Cicirello, <https://www.cicirello.org/>.
+ * Copyright 2018, 2021 Vincent A. Cicirello, <https://www.cicirello.org/>.
  *
  * This file is part of JavaPermutationTools (https://jpt.cicirello.org/).
  *
@@ -26,8 +26,7 @@
  * This class uses Jacobi iteration to compute the eigenvalues and eigenvectors of a symmetric matrix, provided as a 2-D Java array.
  *
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>  
- * @version 1.18.7.1
- * @since 1.3
+ * @version 2.15.2021
  */
 public class JacobiDiagonalization {
 
@@ -150,7 +149,7 @@ private boolean oneIteration(double epsilon) {
 			for (int j = 0; j < N; j++) {
 				if (i==j) continue;
 				double absAij = Math.abs(a[i][j]);
-				if (absAij > 0 && absAij < EPSILON) {
+				if (absAij > 0 && absAij < epsilon) {
 					// zero out any small entries
 					absAij = a[i][j] = 0.0;
 				}
@@ -175,11 +174,13 @@ private boolean oneIteration(double epsilon) {
 			}
 			return true;
 		}
+		/* This appears to be an impossibility, so I'm removing it.
 		// If a_pq is small, simply set entry to 0 and don't rotate.
 		if (a_pq < epsilon) {
 			a[p][q] = 0;
 			return false;
 		}
+		*/
 		// compute some values
 		double f = -a_pq;
 		double g = 0.5*(a[p][p]-a[q][q]);

src/org/cicirello/math/la/MatrixOps.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2019 Vincent A. Cicirello, <https://www.cicirello.org/>.
+ * Copyright 2018-2021 Vincent A. Cicirello, <https://www.cicirello.org/>.
  *
  * This file is part of JavaPermutationTools (https://jpt.cicirello.org/).
  *
@@ -27,8 +27,7 @@
  * represented as 2-D Java arrays.
  *
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>  
- * @version 1.19.5.16
- * @since 1.3
+ * @version 2.13.2021
  */
 public final class MatrixOps {
 
@@ -38,7 +37,8 @@ private MatrixOps() {}
 	/**
 	 * Transpose a square matrix inline.
 	 * @param matrix the matrix to transpose, with result replacing contents of matrix.
-	 * @return The matrix is returned for convenience for passing to other methods requiring a matrix as parameter.  It is the same
+	 * @return The matrix is returned for convenience for passing to other 
+	 * methods requiring a matrix as parameter.  It is the same
 	 *         object reference that was passed as a parameter.
 	 */
 	public static int[][] transposeSquareMatrixInline(int[][] matrix) {
@@ -237,7 +237,7 @@ public static double[][] difference(double[][] A, double[][] B) {
 	public static int[][] product(int[][] A, int[][] B, int[][] C) {
 		if (A.length == 0 && B.length > 0 || A.length > 0 && B.length == 0) {
 			throw new IllegalArgumentException("If either matrix has 0 rows, both must.");
-		} else if (A.length == 0 && B.length == 0) {
+		} else if (A.length == 0) {
 			if (C==null) return new int[0][0];
 			else if (C.length==0) return C;
 			else throw new IllegalArgumentException("C's dimensions are inconsistent with A and B.");
@@ -269,7 +269,7 @@ public static int[][] product(int[][] A, int[][] B, int[][] C) {
 	public static double[][] product(double[][] A, double[][] B, double[][] C) {
 		if (A.length == 0 && B.length > 0 || A.length > 0 && B.length == 0) {
 			throw new IllegalArgumentException("If either matrix has 0 rows, both must.");
-		} else if (A.length == 0 && B.length == 0) {
+		} else if (A.length == 0) {
 			if (C==null) return new double[0][0];
 			else if (C.length==0) return C;
 			else throw new IllegalArgumentException("C's dimensions are inconsistent with A and B.");

src/org/cicirello/math/rand/BTPE.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2019 Vincent A. Cicirello, <https://www.cicirello.org/>.
+ * Copyright 2019-2021 Vincent A. Cicirello, <https://www.cicirello.org/>.
  *
  * This file is part of JavaPermutationTools (https://jpt.cicirello.org/).
  *
@@ -47,9 +47,7 @@
  * one instance of BTPE per thread using ThreadLocal.</p>
  * 
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a> 
- * @version 9.4.2019
- * @since 2.0
- *
+ * @version 2.12.2021
  */
 final class BTPE {
 
@@ -61,14 +59,7 @@ final class BTPE {
 	 * @return A pseudorandom integer from a binomial distribution.
 	 */
 	public static int nextBinomial(int n, double p, Random generator) {
-		BTPE btpe = tl.get();
-		if (btpe == null) {
-			btpe = new BTPE(n, p);
-			tl.set(btpe);
-		} else if (btpe.n != n || btpe.p != p) {
-			btpe.init(n, p);
-		}
-		return btpe.next(generator);
+		return threadLocalInstance(n, p).next(generator);
 	}
 
 	/**
@@ -79,20 +70,28 @@ public static int nextBinomial(int n, double p, Random generator) {
 	 * @return A pseudorandom integer from a binomial distribution.
 	 */
 	public static int nextBinomial(int n, double p, SplittableRandom generator) {
+		return threadLocalInstance(n, p).next(generator);
+	}
+	
+	// We cache constants until n or p changes.  We use ThreadLocal for
+	// cache so that we are thread safe.  Each thread has its own cache.
+	private static final ThreadLocal<BTPE> tl = new ThreadLocal<BTPE>();
+	
+	/*
+	 * Private helper to handle getting the thread local instance of BTPE,
+	 * initializing if necessary, and generating if it doesn't already exist.
+	 */
+	private static BTPE threadLocalInstance(int n, double p) {
 		BTPE btpe = tl.get();
 		if (btpe == null) {
 			btpe = new BTPE(n, p);
 			tl.set(btpe);
 		} else if (btpe.n != n || btpe.p != p) {
 			btpe.init(n, p);
 		}
-		return btpe.next(generator);
+		return btpe;
 	}
 
-	// We cache constants until n or p changes.  We use ThreadLocal for
-	// cache so that we are thread safe.  Each thread has its own cache.
-	private static final ThreadLocal<BTPE> tl = new ThreadLocal<BTPE>();
-	
 	// Variables for caching constants that only change when n or p changes.
 	// Constant cache for BTPE and simple inverse based approach
 	private int n;