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
The following commit(s) were added to refs/heads/geoapi-4.0 by this push: new 906f9ce512 Represent matrix elements as fractions when possible. This enhancement avoids rounding errors with, for example, unit conversions involving a division by 1000. It applies to creations, concatenations and inversions of `MathTransform` where performance is not the primary concern. It does not apply to the coordinate transformations executed by `MathTransform.transform(…)`, where performance matter. 906f9ce512 is described below commit 906f9ce512247d87eb2053c2c78710988247679e Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Mon Jan 9 14:32:51 2023 +0100 Represent matrix elements as fractions when possible. This enhancement avoids rounding errors with, for example, unit conversions involving a division by 1000. It applies to creations, concatenations and inversions of `MathTransform` where performance is not the primary concern. It does not apply to the coordinate transformations executed by `MathTransform.transform(…)`, where performance matter. --- .../org/apache/sis/filter/ArithmeticFunction.java | 6 +- .../sis/internal/referencing/Arithmetic.java | 95 ++++++++++++++++++---- .../referencing/ExtendedPrecisionMatrix.java | 11 --- .../sis/internal/referencing/j2d/AffineMatrix.java | 12 ++- .../sis/referencing/crs/DefaultTemporalCRS.java | 2 +- .../operation/matrix/GeneralMatrix.java | 33 -------- .../referencing/operation/matrix/MatrixSIS.java | 52 ++++++++---- .../sis/referencing/operation/matrix/Solver.java | 6 +- .../operation/matrix/UnmodifiableMatrix.java | 2 +- .../operation/matrix/GeneralMatrixTest.java | 5 +- .../operation/matrix/MatrixTestCase.java | 2 +- .../org/apache/sis/internal/util/Numerics.java | 9 +- .../main/java/org/apache/sis/math/Fraction.java | 66 +++++++++++++-- .../src/main/java/org/apache/sis/util/Numbers.java | 33 ++++---- .../test/java/org/apache/sis/util/NumbersTest.java | 8 +- .../apache/sis/storage/aggregate/GridSlice.java | 5 +- 16 files changed, 219 insertions(+), 128 deletions(-) diff --git a/core/sis-feature/src/main/java/org/apache/sis/filter/ArithmeticFunction.java b/core/sis-feature/src/main/java/org/apache/sis/filter/ArithmeticFunction.java index 694b2f1242..e8ec2b21ae 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/filter/ArithmeticFunction.java +++ b/core/sis-feature/src/main/java/org/apache/sis/filter/ArithmeticFunction.java @@ -292,8 +292,8 @@ abstract class ArithmeticFunction<R> extends BinaryFunction<R,Number,Number> if (BigInteger.ZERO.equals(r[1])) { return r[0]; } else { - return new Fraction(r[1].intValueExact(), right.intValueExact()).add( - new Fraction(r[0].intValueExact(), 1)); + return Fraction.valueOf(r[1].longValueExact(), right.longValueExact()) + .add(new Fraction(r[0].intValueExact(), 1)); } } @@ -303,7 +303,7 @@ abstract class ArithmeticFunction<R> extends BinaryFunction<R,Number,Number> if (left % right == 0) { return r; } else { - return new Fraction(Math.toIntExact(left), Math.toIntExact(right)); + return Fraction.valueOf(left, right); } } } diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/Arithmetic.java b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/Arithmetic.java index 57a56e74aa..22e5201cef 100644 --- a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/Arithmetic.java +++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/Arithmetic.java @@ -16,8 +16,10 @@ */ package org.apache.sis.internal.referencing; +import java.util.function.BiFunction; import java.util.function.BinaryOperator; import org.apache.sis.internal.util.DoubleDouble; +import org.apache.sis.math.Fraction; /** @@ -32,37 +34,43 @@ public enum Arithmetic { /** * The addition operator. */ - ADD(DoubleDouble::add), + ADD(DoubleDouble::add, Fraction::add, Math::addExact), /** * The subtraction operator. */ - SUBTRACT(DoubleDouble::subtract), + SUBTRACT(DoubleDouble::subtract, Fraction::subtract, Math::subtractExact), /** * The multiplication operator. */ - MULTIPLY(DoubleDouble::multiply), + MULTIPLY(DoubleDouble::multiply, Fraction::multiply, Math::multiplyExact), /** * The division operator. */ - DIVIDE(DoubleDouble::divide), + DIVIDE(DoubleDouble::divide, Fraction::divide, Fraction::valueOf), /** * The inverse operation. Operand <var>b</var> is ignored and can be null. */ - INVERSE((a,b) -> a.inverse()), + INVERSE((a,b) -> a.inverse(), + (a,b) -> a.inverse(), + (a,b) -> new Fraction(1, Math.toIntExact(a))), /** * The negation operation. Operand <var>b</var> is ignored and can be null. */ - NEGATE((a,b) -> a.negate()), + NEGATE((a,b) -> a.negate(), + (a,b) -> a.negate(), + (a,b) -> Math.negateExact(a)), /** * The square root operation. Operand <var>b</var> is ignored and can be null. */ - SQRT((a,b) -> a.sqrt()); + SQRT((a,b) -> a.sqrt(), + (a,b) -> DoubleDouble.of(a, false).sqrt(), + (a,b) -> DoubleDouble.of(a).sqrt()); /** * Whether to assume that {@code float} and {@code double} values @@ -75,28 +83,83 @@ public enum Arithmetic { */ private final BinaryOperator<DoubleDouble> onDoubleDouble; + /** + * The arithmetic operation applied on fractions. + * The operation may throw {@link ArithmeticException}, + * in which case {@link #onDoubleDouble} will be used as fallback. + */ + private final BiFunction<Fraction,Fraction,Number> onFraction; + + /** + * The arithmetic operation applied on long integers. + * The operation may throw {@link ArithmeticException}, + * in which case {@link #onDoubleDouble} will be used as fallback. + */ + private final BiFunction<Long,Long,Number> onLong; + /** * Creates a new arithmetic operator. */ - private Arithmetic(final BinaryOperator<DoubleDouble> onDoubleDouble) { + private Arithmetic(final BinaryOperator<DoubleDouble> onDoubleDouble, + final BiFunction<Fraction,Fraction,Number> onFraction, + final BiFunction<Long,Long,Number> onLong) + { this.onDoubleDouble = onDoubleDouble; + this.onFraction = onFraction; + this.onLong = onLong; + } + + /** + * Returns the value of the given number as a long integer if possible. + * If the conversion is not exact, then this method returns {@code null}. + * + * @param element the value to return as a long integer, or {@code null} if zero. + * @return the value as a long integer, or {@code null} if it can not be converted. + */ + private static Long tryLongValue(final Number element) { + if (element == null || element instanceof Long) { + return (Long) element; + } + final long a = element.longValue(); + return (a == element.doubleValue()) ? a : null; } /** * Applies the operation on the given number. * - * @todo Current implementation handles only {@link DoubleDouble} values, - * but more types will be added later. + * @param a the first operand. Shall not be null. + * @param b the second operation. May be null if it does not apply. */ private Number apply(final Number a, final Number b) { - final DoubleDouble result = onDoubleDouble.apply( - DoubleDouble.of(a, DECIMAL), - DoubleDouble.of(b, DECIMAL)); - - if (result.isZero()) return null; + Number result = null; + try { + final Long aLong = tryLongValue(a); + final Long bLong = tryLongValue(b); + if (aLong != null && (bLong != null || b == null)) { + result = onLong.apply(aLong, bLong); + } else { + Fraction aFrac = (a instanceof Fraction) ? (Fraction) a : null; + Fraction bFrac = (b instanceof Fraction) ? (Fraction) b : null; + if (aFrac != null || bFrac != null) { + if (aFrac == null && aLong != null) aFrac = new Fraction(Math.toIntExact(aLong), 1); + if (bFrac == null && bLong != null) bFrac = new Fraction(Math.toIntExact(bLong), 1); + if (aFrac != null && (bFrac != null || b == null)) { + result = onFraction.apply(aFrac, bFrac); + } + } + } + } catch (ArithmeticException e) { + // Ignore and fallback on double-double precision. + } + if (result == null) { + result = onDoubleDouble.apply(DoubleDouble.of(a, DECIMAL), + DoubleDouble.of(b, DECIMAL)); + } + if (ExtendedPrecisionMatrix.isZero(result)) return null; if (result.equals(a)) return a; if (result.equals(b)) return b; - return result; + final Number simplified = tryLongValue(result); + return (simplified != null) ? simplified : result; } /** diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/ExtendedPrecisionMatrix.java b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/ExtendedPrecisionMatrix.java index a0b9872581..0a93024b35 100644 --- a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/ExtendedPrecisionMatrix.java +++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/ExtendedPrecisionMatrix.java @@ -45,17 +45,6 @@ public interface ExtendedPrecisionMatrix extends Matrix { */ Number[] CREATE_IDENTITY = new Number[0]; - /** - * Returns the given matrix as an extended precision matrix if possible, or {@code other} otherwise. - * - * @param source the matrix to cast. - * @param other the value to return if the source is not an instance of {@code ExtendedPrecisionMatrix}. - * @return the given matrix or {@code other}. - */ - static ExtendedPrecisionMatrix castOrElse(final Matrix source, final ExtendedPrecisionMatrix other) { - return (source instanceof ExtendedPrecisionMatrix) ? (ExtendedPrecisionMatrix) source : other; - } - /** * Returns {@code true} if the given number is zero. * This is the criterion used for identifying which {@link Number} elements shall be {@code null}. diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineMatrix.java b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineMatrix.java index 4fa8ddbfea..2d719617bd 100644 --- a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineMatrix.java +++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineMatrix.java @@ -153,6 +153,7 @@ class AffineMatrix extends MatrixSIS implements Serializable, Cloneable { * * @param transform the transform to wrap. * @param elements the elements used for creating the matrix. + * Zero values <em>shall</em> be null. */ ExtendedPrecision(final AffineTransform transform, final Number[] elements) { super(transform); @@ -162,6 +163,7 @@ class AffineMatrix extends MatrixSIS implements Serializable, Cloneable { /** * Returns all matrix elements in row-major order. * Note that this is not the same order than {@link AffineTransform} constructor. + * Zero values <em>shall</em> be null. */ @Override public Number[] getElementAsNumbers(final boolean writable) { @@ -178,15 +180,11 @@ class AffineMatrix extends MatrixSIS implements Serializable, Cloneable { public Number getElementOrNull(final int row, final int column) { ArgumentChecks.ensureBetween("row", 0, AffineTransform2D.DIMENSION, row); ArgumentChecks.ensureBetween("column", 0, AffineTransform2D.DIMENSION, column); - if (row == AffineTransform2D.DIMENSION) { - if (column == AffineTransform2D.DIMENSION) return 1; + if (row != AffineTransform2D.DIMENSION) { + return elements[row * SIZE + column]; } else { - final Number value = elements[row * SIZE + column]; - if (!ExtendedPrecisionMatrix.isZero(value)) { - return value; - } + return (column == AffineTransform2D.DIMENSION) ? 1 : null; } - return null; } /** diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/crs/DefaultTemporalCRS.java b/core/sis-referencing/src/main/java/org/apache/sis/referencing/crs/DefaultTemporalCRS.java index 99b7b49ad0..a8f40457f0 100644 --- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/crs/DefaultTemporalCRS.java +++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/crs/DefaultTemporalCRS.java @@ -231,7 +231,7 @@ public class DefaultTemporalCRS extends AbstractCRS implements TemporalCRS { * If it happens anyway, put the fractional amount of seconds in the converter instead of adding another * field in this class for such very rare situation. Accuracy should be okay since the offset is small. */ - UnitConverter c = Units.converter(null, new Fraction((int) t, MILLIS_PER_SECOND).simplify()); + UnitConverter c = Units.converter(null, Fraction.valueOf(t, MILLIS_PER_SECOND)); toSeconds = c.concatenate(toSeconds); } } 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 3ee241430c..d43410c5b9 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 @@ -20,9 +20,7 @@ import java.util.Arrays; import org.opengis.referencing.operation.Matrix; import org.apache.sis.util.ArgumentChecks; import org.apache.sis.util.ArraysExt; -import org.apache.sis.util.Numbers; import org.apache.sis.internal.util.Numerics; -import org.apache.sis.internal.util.DoubleDouble; import org.apache.sis.internal.referencing.Arithmetic; import org.apache.sis.internal.referencing.ExtendedPrecisionMatrix; @@ -231,24 +229,6 @@ class GeneralMatrix extends MatrixSIS implements ExtendedPrecisionMatrix { throw indexOutOfBounds(row, column); } - /** - * Retrieves the value at the specified row and column of this matrix, rounded to nearest integer. - * - * @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() - * - * @since 1.3 - */ - @Override - public final long getInteger(final int row, final int column) { - final Number value = getElementOrNull(row, column); - return (value != null) ? Numbers.toInteger(value) : 0; - } - /** * Retrieves the value at the specified row and column of this matrix as a {@code Number}. * @@ -449,19 +429,6 @@ class GeneralMatrix extends MatrixSIS implements ExtendedPrecisionMatrix { assert isValid(); } - /** - * Returns the given matrix as an extended precision matrix. - */ - static ExtendedPrecisionMatrix asExtendedPrecision(final Matrix matrix) { - if (matrix instanceof UnmodifiableMatrix) { - return ((UnmodifiableMatrix) matrix).asExtendePrecision(); - } else if (matrix instanceof ExtendedPrecisionMatrix) { - return (ExtendedPrecisionMatrix) matrix; - } else { - return new UnmodifiableMatrix(matrix); - } - } - /** * Returns {@code true} if the specified object is of type {@code GeneralMatrix} and * all of the data members are equal to the corresponding data members in this matrix. 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 3571e1ae7d..89f873624e 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 @@ -27,6 +27,7 @@ import org.apache.sis.util.ArgumentChecks; import org.apache.sis.util.ComparisonMode; import org.apache.sis.util.LenientComparable; import org.apache.sis.util.resources.Errors; +import org.apache.sis.util.Numbers; /** @@ -142,6 +143,19 @@ public abstract class MatrixSIS implements Matrix, LenientComparable, Cloneable, return Matrices.copy(matrix); } + /** + * Returns the given matrix as an extended precision matrix. + */ + static ExtendedPrecisionMatrix asExtendedPrecision(final Matrix matrix) { + if (matrix instanceof UnmodifiableMatrix) { + return ((UnmodifiableMatrix) matrix).asExtendePrecision(); + } else if (matrix instanceof ExtendedPrecisionMatrix) { + return (ExtendedPrecisionMatrix) matrix; + } else { + return new UnmodifiableMatrix(matrix); + } + } + /** * Retrieves the value at the specified row and column if different than zero. * If the value is zero, then this method <em>shall</em> return {@code null}. @@ -170,26 +184,34 @@ public abstract class MatrixSIS implements Matrix, LenientComparable, Cloneable, * @throws ArithmeticException if the value is NaN or overflows integer capacity. * * @since 1.3 + * + * @deprecated Replaced by {@code Numbers.round(getNumber(row, column))}. + * @see Numbers#round(Number) */ + @Deprecated(since="1.4", forRemoval=true) public long getInteger(int row, int 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)); + return Numbers.round(getNumber(row, column)); } /** * Retrieves the value at the specified row and column of this matrix, wrapped in a {@code Number}. * The {@code Number} type depends on the matrix accuracy. * + * <h4>Use case</h4> + * This method may be more accurate than {@link #getElement(int, int)} in some implementations + * when the value is expected to be an integer, for example in conversions of pixel coordinates. + * {@link Number#longValue()} can be more accurate than {@link Number#doubleValue()} because a + * {@code long} may have more significant digits than what a {@code double} can contain. + * For safety against rounding errors and overflows, + * {@link Numbers#round(Number)} should be used instead of {@code Number.longValue()}. + * * @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. * @throws IndexOutOfBoundsException if the specified row or column is out of bounds. * * @see #getElement(int, int) + * @see Numbers#round(Number) */ public Number getNumber(int row, int column) { return getElement(row, column); @@ -265,21 +287,17 @@ public abstract class MatrixSIS implements Matrix, LenientComparable, Cloneable, int dstRow, final int dstCol, int numRow, final int numCol) { - final var exp = ExtendedPrecisionMatrix.castOrElse(source, null); + final var exp = asExtendedPrecision(source); while (--numRow >= 0) { for (int i=0; i<numCol; i++) { - double value; - if (exp != null) { - final Number n = exp.getElementOrNull(srcRow, srcCol + i); - if (n != null) { - setNumber(dstRow, dstCol + i, n); - continue; - } - value = 0; + final int s = srcCol + i; + final int t = dstCol + i; + final Number n = exp.getElementOrNull(srcRow, s); + if (n != null) { + setNumber(dstRow, t, n); } else { - value = source.getElement(srcRow, srcCol + i); + setElement(dstRow, t, source.getElement(srcRow, s)); // Preserve the sign of 0. } - setElement(dstRow, dstCol + i, value); } srcRow++; dstRow++; diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Solver.java b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Solver.java index 0c3f84101b..9662e1c114 100644 --- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Solver.java +++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Solver.java @@ -124,7 +124,7 @@ final class Solver implements ExtendedPrecisionMatrix { // Not C if (numCol != size) { throw new NoninvertibleMatrixException(Resources.format(Resources.Keys.NonInvertibleMatrix_2, size, numCol)); } - return solve(GeneralMatrix.asExtendedPrecision(X), IDENTITY, size, size); + return solve(MatrixSIS.asExtendedPrecision(X), IDENTITY, size, size); } /** @@ -143,8 +143,8 @@ final class Solver implements ExtendedPrecisionMatrix { // Not C } final int innerSize = Y.getNumCol(); GeneralMatrix.ensureNumRowMatch(size, Y.getNumRow(), innerSize); - return solve(GeneralMatrix.asExtendedPrecision(X), - GeneralMatrix.asExtendedPrecision(Y), + return solve(MatrixSIS.asExtendedPrecision(X), + MatrixSIS.asExtendedPrecision(Y), size, innerSize); } diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/UnmodifiableMatrix.java b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/UnmodifiableMatrix.java index 4bd0b23f31..6ea73d1e81 100644 --- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/UnmodifiableMatrix.java +++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/UnmodifiableMatrix.java @@ -53,7 +53,7 @@ final class UnmodifiableMatrix extends MatrixSIS implements ExtendedPrecisionMat * The returned matrix shall be considered read-only. */ final ExtendedPrecisionMatrix asExtendePrecision() { - return ExtendedPrecisionMatrix.castOrElse(matrix, this); + return (matrix instanceof ExtendedPrecisionMatrix) ? (ExtendedPrecisionMatrix) matrix : this; } /** 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 eb52a5f499..89614a3865 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 @@ -60,8 +60,8 @@ public final class GeneralMatrixTest extends MatrixTestCase { } /** - * Tests {@link GeneralMatrix#getNumber(int, int)} and {@link GeneralMatrix#getInteger(int, int)} - * using a value which cannot be stored accurately in a {@code double} type. + * Tests {@link GeneralMatrix#getNumber(int, int)} using a value which + * cannot be stored accurately in a {@code double} type. */ @Test public void testExtendedPrecision() { @@ -73,7 +73,6 @@ public final class GeneralMatrixTest extends MatrixTestCase { matrix.setNumber(0, 0, ddval); assertEquals(value, ddval.longValue()); assertEquals(ddval, matrix.getNumber (0, 0)); - assertEquals(value, matrix.getInteger(0, 0)); validateImplementation(matrix); } 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 86585e9a3b..7376c5c599 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 @@ -162,7 +162,7 @@ public abstract class MatrixTestCase extends TestCase { } final int numRow = matrix.getNumRow(); final int numCol = matrix.getNumCol(); - final var extend = GeneralMatrix.asExtendedPrecision(matrix); + final var extend = MatrixSIS.asExtendedPrecision(matrix); final Number[] numbers = extend.getElementAsNumbers(false); final double[] elements = (matrix instanceof MatrixSIS) ? ((MatrixSIS) matrix).getElements() : null; assertEquals("getElementAsNumbers", numRow * numCol, numbers.length); diff --git a/core/sis-utility/src/main/java/org/apache/sis/internal/util/Numerics.java b/core/sis-utility/src/main/java/org/apache/sis/internal/util/Numerics.java index 8437a7a3db..25d2a9dc9b 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/internal/util/Numerics.java +++ b/core/sis-utility/src/main/java/org/apache/sis/internal/util/Numerics.java @@ -329,12 +329,9 @@ public final class Numerics extends Static { * @return the fraction as a {@link Fraction} or {@link Double} object. */ public static Number fraction(long numerator, long denominator) { - final int simplify = Math.min(Long.numberOfTrailingZeros(numerator), Long.numberOfTrailingZeros(denominator)); - final int num = (int) (numerator >>= simplify); - final int den = (int) (denominator >>= simplify); - if (num == numerator && den == denominator) { - return new Fraction(num, den).unique(); - } else { + try { + return Fraction.valueOf(numerator, denominator).unique(); + } catch (ArithmeticException e) { return valueOf(numerator / (double) denominator); } } diff --git a/core/sis-utility/src/main/java/org/apache/sis/math/Fraction.java b/core/sis-utility/src/main/java/org/apache/sis/math/Fraction.java index 33a78f5ed6..9c41ea3bd2 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/math/Fraction.java +++ b/core/sis-utility/src/main/java/org/apache/sis/math/Fraction.java @@ -72,6 +72,22 @@ public final class Fraction extends Number implements Comparable<Fraction>, Seri this.denominator = denominator; } + /** + * Returns the given fraction after simplification. + * If the numerator or denominator is still too large for 32 bit integer after simplification, + * then an {@link ArithmeticException} is thrown. + * + * @param numerator numerator of the fraction to return. + * @param denominator denominator of the fraction to return. + * @return the simplified fraction. + * @throws ArithmeticException if the numerator and denominator can not be represented by 32 bit integers. + * + * @since 1.4 + */ + public static Fraction valueOf(final long numerator, final long denominator) { + return simplify(null, numerator, denominator); + } + /** * Converts the given IEEE 754 double-precision value to a fraction. If successful, this method returns a fraction * such as {@link #doubleValue()} is equal to the given value in the sense of {@link Double#equals(Object)}: @@ -208,21 +224,22 @@ public final class Fraction extends Number implements Comparable<Fraction>, Seri /** * Returns a fraction equivalent to {@code num} / {@code den} after simplification. - * If the simplified fraction is equal to {@code this}, then this method returns {@code this}. + * If the simplified fraction is equal to {@code f}, then this method returns {@code f}. * * <p>The arguments given to this method are the results of multiplications and additions of {@code int} values. * This method fails if any argument value is {@link Long#MIN_VALUE} because that value cannot be made positive. * However, it should never happen. Even in the worst scenario:</p> * - * {@prefomat java + * {@snippet lang="java" : * long n = Math.multiplyFull(Integer.MIN_VALUE, Integer.MAX_VALUE); * n += n; - * } + * } * * Above result still slightly smaller in magnitude than {@code Long.MIN_VALUE}. */ private static Fraction simplify(final Fraction f, long num, long den) { if (num == Long.MIN_VALUE || den == Long.MIN_VALUE) { + // TODO: replace by use of Math.absExact(…) in JDK15. throw new ArithmeticException(Errors.format(Errors.Keys.IntegerOverflow_1, Long.SIZE)); } if (num == 0) { @@ -262,12 +279,30 @@ public final class Fraction extends Number implements Comparable<Fraction>, Seri : new Fraction(Math.toIntExact(num), Math.toIntExact(den)); } + /** + * Returns the inverse value of this fraction. + * This method does not simplify the fraction. + * + * @return the result of {@code 1/this}. + * @throws ArithmeticException if the result overflows. + * + * @since 1.4 + * + * @see #divide(Fraction) + */ + public Fraction inverse() { + if (numerator == denominator) return this; + return new Fraction(denominator, numerator); + } + /** * Returns the negative value of this fraction. * This method does not simplify the fraction. * * @return the result of {@code -this}. * @throws ArithmeticException if the result overflows. + * + * @see #subtract(Fraction) */ public Fraction negate() { int n = numerator; @@ -302,6 +337,8 @@ public final class Fraction extends Number implements Comparable<Fraction>, Seri * @param other the fraction to subtract from this fraction. * @return the simplified result of {@code this} - {@code other}. * @throws ArithmeticException if the result overflows. + * + * @see #negate() */ public Fraction subtract(final Fraction other) { // Intermediate result must be computed in a type wider that the 'numerator' and 'denominator' type. @@ -328,6 +365,8 @@ public final class Fraction extends Number implements Comparable<Fraction>, Seri * @param other the fraction by which to divide this fraction. * @return the simplified result of {@code this} ∕ {@code other}. * @throws ArithmeticException if the result overflows. + * + * @see #inverse() */ public Fraction divide(final Fraction other) { return simplify(this, Math.multiplyFull(numerator, other.denominator), @@ -433,17 +472,27 @@ public final class Fraction extends Number implements Comparable<Fraction>, Seri /** * Returns this fraction rounded toward zero. + * If the fraction value {@linkplain #isNaN() is NaN}, then this method returns 0. + * If the fraction value is positive or negative infinity, then this method returns + * {@link Long#MAX_VALUE} or {@link Long#MIN_VALUE} respectively. * * @return this fraction rounded toward zero. */ @Override public long longValue() { - return intValue(); + if (denominator != 0) { + return numerator / denominator; + } + if (numerator < 0) return Long.MIN_VALUE; + if (numerator > 0) return Long.MAX_VALUE; + return 0; } /** * Returns this fraction rounded toward zero. - * If the fraction value is NaN, then this method returns 0. + * If the fraction value {@linkplain #isNaN() is NaN}, then this method returns 0. + * If the fraction value is positive or negative infinity, then this method returns + * {@link Integer#MAX_VALUE} or {@link Integer#MIN_VALUE} respectively. * * @return {@link #numerator} / {@link #denominator} rounded toward zero. * @@ -453,7 +502,12 @@ public final class Fraction extends Number implements Comparable<Fraction>, Seri */ @Override public int intValue() { - return isNaN() ? 0 : numerator / denominator; + if (denominator != 0) { + return numerator / denominator; + } + if (numerator < 0) return Integer.MIN_VALUE; + if (numerator > 0) return Integer.MAX_VALUE; + return 0; } /* diff --git a/core/sis-utility/src/main/java/org/apache/sis/util/Numbers.java b/core/sis-utility/src/main/java/org/apache/sis/util/Numbers.java index 46979ce218..6ae0d9ebdc 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/util/Numbers.java +++ b/core/sis-utility/src/main/java/org/apache/sis/util/Numbers.java @@ -179,7 +179,7 @@ public final class Numbers extends Static { * @return {@code true} if {@code type} is an integer type. * * @see #isFloat(Class) - * @see #toInteger(Number) + * @see #round(Number) */ public static boolean isInteger(final Class<?> type) { final Numbers mapping = MAPPING.get(type); @@ -228,41 +228,44 @@ public final class Numbers extends Static { } /** - * Returns the value of the given number as a {@code long} integer. - * This method makes the following choice based on the number type: + * Returns the value of the given number rounded to nearest {@code long} integer. + * This method is intended for calculations where fractional parts are rounding errors. + * An {@link ArithmeticException} is thrown in the following cases: * * <ul> - * <li>If {@link BigDecimal} or {@link BigInteger}, delegate to its {@code longValueExact()} method.</li> - * <li>If {@linkplain #isInteger(Class) an integer}, delegate to {@link Number#longValue()}.</li> - * <li>Otherwise delegate to {@link Number#doubleValue()}, round the value and verify that - * the result is representable as a {@code long}.</li> + * <li>If the floating point value is NaN or positive or negative infinity.</li> + * <li>If the value overflows the capacity of {@value Long#SIZE} bits integers.</li> + * <li>If the number is a {@link BigDecimal} with a non-zero fractional part.</li> * </ul> * + * The justification for the last case is that {@link BigDecimal} is used when calculations + * should be exact in base 10. In such case, a fractional part would not be a rounding error. + * * @param value the value to return as a long integer. - * @return the value as a long integer, possibly rounded to nearest integer. + * @return the value rounded to nearest long integer. * @throws NullPointerException if the given number is {@code null}. * @throws ArithmeticException if the value can not be represented as a long integer. * + * @see Math#round(double) * @see BigDecimal#longValueExact() * @see BigInteger#longValueExact() * * @since 1.4 */ - public static long toInteger(final Number value) { + public static long round(final Number value) { final Numbers mapping = MAPPING.get(value.getClass()); - if (mapping != null) { +asLong: if (mapping != null) { switch (mapping.ordinal) { - case DOUBLE_DOUBLE: return value.longValue(); // Does rounding. + case DOUBLE_DOUBLE: break; case BIG_DECIMAL: return ((BigDecimal) value).longValueExact(); case BIG_INTEGER: return ((BigInteger) value).longValueExact(); + default: if (!mapping.isInteger) break asLong; } - if (mapping.isInteger) return value.longValue(); + return value.longValue(); // Does rounding in `DoubleDouble` case. } final double v = value.doubleValue(); final long n = Math.round(v); - if (Math.abs(v - n) <= 0.5) { - return n; - } + if (Math.abs(v - n) <= 0.5) return n; throw new ArithmeticException(Errors.format(Errors.Keys.CanNotConvertValue_2, value, Long.TYPE)); } diff --git a/core/sis-utility/src/test/java/org/apache/sis/util/NumbersTest.java b/core/sis-utility/src/test/java/org/apache/sis/util/NumbersTest.java index 2a094a02dc..b7618a8207 100644 --- a/core/sis-utility/src/test/java/org/apache/sis/util/NumbersTest.java +++ b/core/sis-utility/src/test/java/org/apache/sis/util/NumbersTest.java @@ -83,13 +83,13 @@ public final class NumbersTest extends TestCase { } /** - * Tests {@link Numbers#toInteger(Number)}. + * Tests {@link Numbers#round(Number)}. */ @Test - public void testToInteger() { - assertEquals(123456, Numbers.toInteger(123456.2f)); + public void testRound() { + assertEquals(123456, Numbers.round(123456.2f)); try { - Numbers.toInteger(Long.MAX_VALUE * 3d); + Numbers.round(Long.MAX_VALUE * 3d); fail("Expected ArithmeticException"); } catch (ArithmeticException e) { assertNotNull(e.getMessage()); diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/aggregate/GridSlice.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/aggregate/GridSlice.java index 5b27a0f172..489af18d74 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/storage/aggregate/GridSlice.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/aggregate/GridSlice.java @@ -30,6 +30,7 @@ import org.apache.sis.coverage.grid.GridGeometry; import org.apache.sis.coverage.grid.GridExtent; import org.apache.sis.internal.util.Numerics; import org.apache.sis.internal.util.Strings; +import org.apache.sis.util.Numbers; /** @@ -79,13 +80,14 @@ final class GridSlice { * * @param groupToSlice conversion from source coordinates of {@link GroupByTransform#gridToCRS} * to grid coordinates of {@link #geometry}. + * @throws ArithmeticException if a translation term is NaN or overflows {@code long} integer capacity. * * @see #getOffset(Map) */ private void setOffset(final MatrixSIS groupToSlice) { final int i = groupToSlice.getNumCol() - 1; for (int j=0; j<offset.length; j++) { - offset[j] = groupToSlice.getInteger(j, i); + offset[j] = Numbers.round(groupToSlice.getNumber(j, i)); } } @@ -132,6 +134,7 @@ final class GridSlice { * @param strategy algorithm to apply when more than one grid coverage can be found at the same grid index. * @return group of objects associated to the given transform (never null). * @throws NoninvertibleTransformException if the transform is not invertible. + * @throws ArithmeticException if a translation term is NaN or overflows {@code long} integer capacity. */ final GroupByTransform getList(final List<GroupByCRS<GroupByTransform>> groups, final MergeStrategy strategy) throws NoninvertibleTransformException