This is an automated email from the ASF dual-hosted git repository.
desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git
commit 4f11c0244de664dce82d95aebebd3eac72b9489a
Author: Martin Desruisseaux <martin.desruisse...@geomatys.com>
AuthorDate: Fri Dec 30 14:14:38 2022 +0100
Add more verifications of `GeneralMatrix` validity.
Fix a bug in `GeneralMatrix.setElements(double[])`
identified by above-cited verifications.
---
.../operation/matrix/GeneralMatrix.java | 62 ++++++++++++++++++----
.../sis/referencing/operation/matrix/Matrices.java | 5 +-
.../referencing/operation/matrix/MatrixSIS.java | 10 +++-
.../referencing/operation/matrix/package-info.java | 2 +-
.../operation/matrix/GeneralMatrixTest.java | 5 +-
.../operation/matrix/MatrixTestCase.java | 27 +++++-----
.../org/apache/sis/internal/util/DoubleDouble.java | 14 +++--
.../apache/sis/internal/util/DoubleDoubleTest.java | 8 +--
8 files changed, 93 insertions(+), 40 deletions(-)
diff --git
a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/GeneralMatrix.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/GeneralMatrix.java
index 90a4fcc596..0fba129a0c 100644
---
a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/GeneralMatrix.java
+++
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/GeneralMatrix.java
@@ -36,7 +36,7 @@ import
org.apache.sis.internal.referencing.ExtendedPrecisionMatrix;
* the {@link DoubleDouble#error}.
*
* @author Martin Desruisseaux (IRD, Geomatys)
- * @version 1.3
+ * @version 1.4
*
* @see Matrices#createDiagonal(int, int)
*
@@ -125,6 +125,7 @@ class GeneralMatrix extends MatrixSIS implements
ExtendedPrecisionMatrix {
this.numRow = (short) numRow;
this.numCol = (short) numCol;
this.elements = elements.clone();
+ assert isValid();
}
/**
@@ -145,6 +146,7 @@ class GeneralMatrix extends MatrixSIS implements
ExtendedPrecisionMatrix {
elements = new double[numRow * numCol];
getElements(matrix, numRow, numCol, elements);
}
+ assert isValid();
}
/**
@@ -154,6 +156,34 @@ class GeneralMatrix extends MatrixSIS implements
ExtendedPrecisionMatrix {
numRow = matrix.numRow;
numCol = matrix.numCol;
elements = matrix.elements.clone();
+ assert isValid();
+ }
+
+ /**
+ * Verifies that this matrix is well-formed. This method verifies that the
matrix size is valid,
+ * that the {@link #elements} array is non-null and has a valid length,
and that all error terms
+ * are smaller than 1 ULP of the corresponding matrix element value.
+ *
+ * @return whether this matrix is well-formed.
+ * @throws NullPointerException if the {@link #elements} array is null.
+ * @throws AssertionError (thrown by {@link
DoubleDouble#DoubleDouble(double, double)} constructor)
+ * if an error term is not smaller than the corresponding matrix
element value.
+ */
+ private boolean isValid() {
+ final int numRow = this.numRow;
+ final int numCol = this.numCol;
+ final int length = elements.length;
+ int i = numRow * numCol; // Cannot overflow.
+ if ((numRow | numCol) < 0 || (length != i && length != i*2) ||
+ (numRow != numCol) != (this instanceof NonSquareMatrix))
+ {
+ return false;
+ }
+ boolean isValid = true;
+ while (--i >= 0) {
+ isValid &= Numerics.equals(getNumber(i / numCol, i %
numCol).doubleValue(), elements[i]);
+ }
+ return isValid;
}
/**
@@ -267,6 +297,7 @@ class GeneralMatrix extends MatrixSIS implements
ExtendedPrecisionMatrix {
* @param row the row index, from 0 inclusive to {@link #getNumRow()}
exclusive.
* @param column the column index, from 0 inclusive to {@link
#getNumCol()} exclusive.
* @return the current value at the given row and column, rounded to
nearest integer.
+ * @throws ArithmeticException if the value is NaN or overflows integer
capacity.
*
* @see DoubleDouble#longValue()
*
@@ -276,15 +307,18 @@ class GeneralMatrix extends MatrixSIS implements
ExtendedPrecisionMatrix {
public long getInteger(int row, int column) {
if (row >= 0 && row < numRow && column >= 0 && column < numCol) {
int i = row * numCol + column;
- long value = Math.round(elements[i]);
+ final double value = elements[i];
+ long r = Math.round(value);
i += numRow * numCol;
if (i < elements.length) {
- value += (long) elements[i]; // Really want rounding
toward zero.
+ r += (long) elements[i]; // Really want rounding
toward zero.
}
- return value;
- } else {
- throw indexOutOfBounds(row, column);
+ if (Math.abs(r - value) <= 0.5) {
+ return r;
+ }
+ throw new
ArithmeticException(Errors.format(Errors.Keys.CanNotConvertValue_2, value,
Long.TYPE));
}
+ throw indexOutOfBounds(row, column);
}
/**
@@ -395,7 +429,8 @@ class GeneralMatrix extends MatrixSIS implements
ExtendedPrecisionMatrix {
}
/**
- * {@inheritDoc}
+ * Returns a copy of all matrix elements in a flat, row-major (column
indices vary fastest) array.
+ * The returned array does <em>not</em> include error terms used in
double-double arithmetic.
*/
@Override
public final double[] getElements() {
@@ -412,15 +447,17 @@ class GeneralMatrix extends MatrixSIS implements
ExtendedPrecisionMatrix {
}
/**
- * {@inheritDoc}
+ * Sets all matrix elements from a flat, row-major (column indices vary
fastest) array.
+ * The given array shall not contain error terms. The error terms will be
set to default values.
*/
@Override
public final void setElements(final double[] newValues) {
ensureLengthMatch(numRow*numCol, newValues);
System.arraycopy(newValues, 0, elements, 0, newValues.length);
if (elements.length != newValues.length) {
- inferErrors(newValues);
+ inferErrors(elements);
}
+ assert isValid();
}
/**
@@ -436,7 +473,7 @@ class GeneralMatrix extends MatrixSIS implements
ExtendedPrecisionMatrix {
* </ul>
*
* @param newValues the new matrix elements in a row-major array.
- * @return {@code true} if at leat one {@link DoubleDouble} instance has
been found, in which case all
+ * @return {@code true} if at least one {@link DoubleDouble} instance has
been found, in which case all
* errors terms have been initialized, or {@code false} otherwise,
in which case no error term
* has been initialized (this is a <cite>all or nothing</cite>
operation).
* @throws IllegalArgumentException if the given array does not have the
expected length.
@@ -486,6 +523,9 @@ class GeneralMatrix extends MatrixSIS implements
ExtendedPrecisionMatrix {
}
elements[i + length] = error;
}
+ if (isExtended) {
+ assert isValid();
+ }
return isExtended;
}
@@ -513,6 +553,7 @@ class GeneralMatrix extends MatrixSIS implements
ExtendedPrecisionMatrix {
} else {
super.setMatrix(matrix);
}
+ assert isValid();
}
/**
@@ -662,6 +703,7 @@ class GeneralMatrix extends MatrixSIS implements
ExtendedPrecisionMatrix {
sum.storeTo(elements, k++, errorOffset);
}
}
+ assert isValid();
}
/**
diff --git
a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Matrices.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Matrices.java
index 6a74205416..c1f50e882c 100644
---
a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Matrices.java
+++
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Matrices.java
@@ -212,8 +212,9 @@ public final class Matrices extends Static {
* Below is an intantionally undocumented feature. We use those
sentinel values as a way to create
* matrices with extended precision without exposing our double-double
arithmetic in public API.
*/
- if (elements == ExtendedPrecisionMatrix.IDENTITY || elements ==
ExtendedPrecisionMatrix.ZERO) {
- return GeneralMatrix.createExtendedPrecision(numRow, numCol,
elements == ExtendedPrecisionMatrix.IDENTITY);
+ final boolean setToIdentity = (elements ==
ExtendedPrecisionMatrix.IDENTITY);
+ if (setToIdentity || elements == ExtendedPrecisionMatrix.ZERO) {
+ return GeneralMatrix.createExtendedPrecision(numRow, numCol,
setToIdentity);
}
final GeneralMatrix matrix =
GeneralMatrix.createExtendedPrecision(numRow, numCol, false);
if (matrix.setElements(elements)) {
diff --git
a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/MatrixSIS.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/MatrixSIS.java
index c3d9f19e49..ab9afb41ec 100644
---
a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/MatrixSIS.java
+++
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/MatrixSIS.java
@@ -55,7 +55,7 @@ import org.apache.sis.util.resources.Errors;
* </ul>
*
* @author Martin Desruisseaux (IRD, Geomatys)
- * @version 1.3
+ * @version 1.4
*
* @see Matrices
*
@@ -170,11 +170,17 @@ public abstract class MatrixSIS implements Matrix,
LenientComparable, Cloneable,
* @param row the row index, from 0 inclusive to {@link #getNumRow()}
exclusive.
* @param column the column index, from 0 inclusive to {@link
#getNumCol()} exclusive.
* @return the current value at the given row and column, rounded to
nearest integer.
+ * @throws ArithmeticException if the value is NaN or overflows integer
capacity.
*
* @since 1.3
*/
public long getInteger(int row, int column) {
- return Math.round(getElement(row, column));
+ final double value = getElement(row, column);
+ final long r = Math.round(value);
+ if (Math.abs(r - value) <= 0.5) {
+ return r;
+ }
+ throw new
ArithmeticException(Errors.format(Errors.Keys.CanNotConvertValue_2, value,
Long.TYPE));
}
/**
diff --git
a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/package-info.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/package-info.java
index b424dae044..ed8b445699 100644
---
a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/package-info.java
+++
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/package-info.java
@@ -69,7 +69,7 @@
* Like SIS, Vecmath is optimized for small matrices of interest for 2D and 3D
graphics.</p>
*
* @author Martin Desruisseaux (IRD, Geomatys)
- * @version 1.3
+ * @version 1.4
* @since 0.4
*/
package org.apache.sis.referencing.operation.matrix;
diff --git
a/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/matrix/GeneralMatrixTest.java
b/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/matrix/GeneralMatrixTest.java
index 7321fae51e..2febade1b6 100644
---
a/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/matrix/GeneralMatrixTest.java
+++
b/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/matrix/GeneralMatrixTest.java
@@ -66,7 +66,7 @@ public final class GeneralMatrixTest extends MatrixTestCase {
@Test
public void testExtendedPrecision() {
final long value = 1000000000000010000L;
- assertNotEquals(value, Math.round((double) value));
+ assertNotEquals(value, StrictMath.round((double) value));
final GeneralMatrix m = new GeneralMatrix(1, 1, false, 2);
final DoubleDouble ddval = new DoubleDouble((Long) value);
m.setNumber(0, 0, ddval);
@@ -155,7 +155,6 @@ public final class GeneralMatrixTest extends MatrixTestCase
{
@Test
public void testTranslateVector() {
testTranslateVector(new GeneralMatrix(3, 3, true, 1)); // Double
precision
-// testTranslateVector(new GeneralMatrix(3, 3, true, 2)); //
Double-double precision
- // TODO: revisit commented-out test after using Math.fma with JDK9.
+ testTranslateVector(new GeneralMatrix(3, 3, true, 2)); //
Double-double precision
}
}
diff --git
a/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/matrix/MatrixTestCase.java
b/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/matrix/MatrixTestCase.java
index c46f9dd025..120667f6c3 100644
---
a/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/matrix/MatrixTestCase.java
+++
b/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/matrix/MatrixTestCase.java
@@ -26,6 +26,7 @@ import org.apache.sis.test.TestUtilities;
import org.apache.sis.test.DependsOnMethod;
import org.junit.Test;
+import static java.lang.StrictMath.*;
import static org.apache.sis.test.Assert.*;
@@ -168,7 +169,7 @@ public abstract class MatrixTestCase extends TestCase {
assertEquals(name, e, actual.getNumber(j,i).doubleValue(),
tolerance);
if (tolerance != STRICT && statistics != null) {
synchronized (statistics) {
- statistics.accept(StrictMath.abs(e - a));
+ statistics.accept(abs(e - a));
}
}
}
@@ -194,7 +195,7 @@ public abstract class MatrixTestCase extends TestCase {
private static void assertEqualsRelative(final String message, final
double expected,
final MatrixSIS matrix, final int row, final int column)
{
- assertEquals(message, expected, matrix.getElement(row, column),
StrictMath.abs(expected) * 1E-12);
+ assertEquals(message, expected, matrix.getElement(row, column),
abs(expected) * 1E-12);
}
/**
@@ -204,7 +205,7 @@ public abstract class MatrixTestCase extends TestCase {
*/
private double nextNonZeroRandom() {
double value = random.nextDouble() * 200 - 100;
- value += StrictMath.copySign(0.001, value);
+ value += copySign(0.001, value);
if (random.nextBoolean()) {
value = 1 / value;
}
@@ -399,7 +400,7 @@ public abstract class MatrixTestCase extends TestCase {
final double e = matrix.getElement(j, i);
sum += e*e;
}
- return StrictMath.sqrt(sum);
+ return sqrt(sum);
}
/**
@@ -525,8 +526,8 @@ public abstract class MatrixTestCase extends TestCase {
if ((n % 10) == 0) {
setRandomValues(at, matrix);
}
- at.translate(vector[0] = Math.fma(random.nextDouble(), 50, -25),
- vector[1] = Math.fma(random.nextDouble(), 50, -25));
+ at.translate(vector[0] = fma(random.nextDouble(), 50, -25),
+ vector[1] = fma(random.nextDouble(), 50, -25));
matrix.translate(vector);
assertMatrixEquals("translate", AffineTransforms2D.toMatrix(at),
matrix, TOLERANCE);
}
@@ -536,10 +537,10 @@ public abstract class MatrixTestCase extends TestCase {
* Sets random values in the given affine transform and a copy of those
values in the given matrix.
*/
private void setRandomValues(final AffineTransform at, final MatrixSIS
matrix) {
- at.setToRotation(random.nextDouble() * StrictMath.PI);
+ at.setToRotation(random.nextDouble() * PI);
at.scale(nextNonZeroRandom(), nextNonZeroRandom());
- at.translate(Math.fma(random.nextDouble(), 100, - 50),
- Math.fma(random.nextDouble(), 100, - 50));
+ at.translate(fma(random.nextDouble(), 100, - 50),
+ fma(random.nextDouble(), 100, - 50));
matrix.setElements(new double[] {
at.getScaleX(), at.getShearX(), at.getTranslateX(),
at.getShearY(), at.getScaleY(), at.getTranslateY(),
@@ -564,8 +565,8 @@ public abstract class MatrixTestCase extends TestCase {
if ((n % 10) == 0) {
setRandomValues(at, matrix);
}
- vector[0] = Math.fma(random.nextDouble(), 50, -25);
- vector[1] = Math.fma(random.nextDouble(), 50, -25);
+ vector[0] = fma(random.nextDouble(), 50, -25);
+ vector[1] = fma(random.nextDouble(), 50, -25);
vector[2] = 1;
final double[] result = matrix.multiply(vector); // The
result to verify.
at.transform(vector, 0, vector, 0, 1); // The
expected result.
@@ -596,7 +597,7 @@ public abstract class MatrixTestCase extends TestCase {
final int nx = random.nextInt(8) + 1;
elements = new double[numCol * nx];
for (int k=0; k<elements.length; k++) {
- elements[k] = Math.fma(random.nextDouble(), -10, 8);
+ elements[k] = fma(random.nextDouble(), -10, 8);
}
final Matrix referenceArg = new Matrix(elements, nx).transpose();
final MatrixSIS matrixArg = Matrices.create(numCol, nx, elements);
@@ -636,7 +637,7 @@ public abstract class MatrixTestCase extends TestCase {
final int nx = random.nextInt(8) + 1;
elements = new double[numCol * nx];
for (int k=0; k<elements.length; k++) {
- elements[k] = Math.fma(random.nextDouble(), -10, 8);
+ elements[k] = fma(random.nextDouble(), -10, 8);
}
final Matrix referenceArg = new Matrix(elements, nx).transpose();
final MatrixSIS matrixArg = Matrices.create(numCol, nx, elements);
diff --git
a/core/sis-utility/src/main/java/org/apache/sis/internal/util/DoubleDouble.java
b/core/sis-utility/src/main/java/org/apache/sis/internal/util/DoubleDouble.java
index af78f73076..6cf925f1ba 100644
---
a/core/sis-utility/src/main/java/org/apache/sis/internal/util/DoubleDouble.java
+++
b/core/sis-utility/src/main/java/org/apache/sis/internal/util/DoubleDouble.java
@@ -347,11 +347,15 @@ public final class DoubleDouble extends Number {
public static double errorForWellKnownValue(final double value) {
if (DISABLED) return 0;
final int i = Arrays.binarySearch(VALUES, Math.abs(value));
+ final double error;
if (i >= 0) {
- return MathFunctions.xorSign(ERRORS[i], value);
+ error = MathFunctions.xorSign(ERRORS[i], value);
+ } else {
+ final double delta =
DecimalFunctions.deltaForDoubleToDecimal(value);
+ error = Double.isNaN(delta) ? 0 : delta;
}
- final double delta = DecimalFunctions.deltaForDoubleToDecimal(value);
- return Double.isNaN(delta) ? 0 : delta;
+ assert !(Math.abs(error) >= Math.ulp(value)) : value; // Use !
for being tolerant to NaN.
+ return error;
}
/**
@@ -834,8 +838,8 @@ public final class DoubleDouble extends Number {
final double thisValue = this.value;
final double thisError = this.error;
setToProduct(thisValue, otherValue);
- error += otherError * thisValue;
- error += otherValue * thisError;
+ error = Math.fma(otherError, thisValue, error);
+ error = Math.fma(otherValue, thisError, error);
normalize();
}
diff --git
a/core/sis-utility/src/test/java/org/apache/sis/internal/util/DoubleDoubleTest.java
b/core/sis-utility/src/test/java/org/apache/sis/internal/util/DoubleDoubleTest.java
index 9b15b57586..f54ab22923 100644
---
a/core/sis-utility/src/test/java/org/apache/sis/internal/util/DoubleDoubleTest.java
+++
b/core/sis-utility/src/test/java/org/apache/sis/internal/util/DoubleDoubleTest.java
@@ -71,7 +71,7 @@ public final class DoubleDoubleTest extends TestCase {
* in order to change only the exponent part of the IEEE representation.
*/
private double nextRandom() {
- return random.nextDouble() * 2048 - 1024;
+ return fma(random.nextDouble(), 2048, -1024);
}
/**
@@ -224,7 +224,7 @@ public final class DoubleDoubleTest extends TestCase {
nextRandom(dd);
nextRandom(op);
final BigDecimal expected = toBigDecimal(dd).add(toBigDecimal(op));
- dd.add(op); // Must be after 'expected' computation.
+ dd.add(op); // Must be after `expected` computation.
assertExtendedEquals(expected, dd, ADD_TOLERANCE_FACTOR);
}
}
@@ -241,7 +241,7 @@ public final class DoubleDoubleTest extends TestCase {
nextRandom(dd);
nextRandom(op);
final BigDecimal expected =
toBigDecimal(dd).multiply(toBigDecimal(op), MathContext.DECIMAL128);
- dd.multiply(op); // Must be after 'expected' computation.
+ dd.multiply(op); // Must be after `expected` computation.
assertExtendedEquals(expected, dd, PRODUCT_TOLERANCE_FACTOR);
}
}
@@ -258,7 +258,7 @@ public final class DoubleDoubleTest extends TestCase {
nextRandom(dd);
nextRandom(op);
final BigDecimal expected =
toBigDecimal(dd).divide(toBigDecimal(op), MathContext.DECIMAL128);
- dd.divide(op); // Must be after 'expected' computation.
+ dd.divide(op); // Must be after `expected` computation.
assertExtendedEquals(expected, dd, PRODUCT_TOLERANCE_FACTOR);
}
}
