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 8b80a97 Identity unit conversion should preserve negative zero. When
the value is used in a map projection parameter, the sign has implications in
the concatenated transforms chain. The final result is numerically equivalent,
but intermediate steps may differ depending on the parameter sign.
8b80a97 is described below
commit 8b80a971cdea37a107ce1c763e758775da669fb2
Author: Martin Desruisseaux <martin.desruisse...@geomatys.com>
AuthorDate: Thu Sep 19 15:39:00 2019 +0200
Identity unit conversion should preserve negative zero.
When the value is used in a map projection parameter, the sign has
implications in the concatenated transforms chain.
The final result is numerically equivalent, but intermediate steps may
differ depending on the parameter sign.
---
.../org/apache/sis/measure/AbstractConverter.java | 2 +-
.../apache/sis/measure/ConcatenatedConverter.java | 2 +-
.../org/apache/sis/measure/ConventionalUnit.java | 4 +-
.../org/apache/sis/measure/IdentityConverter.java | 109 +++++++++++++++++++++
.../org/apache/sis/measure/LinearConverter.java | 49 +++++----
.../java/org/apache/sis/measure/SystemUnit.java | 4 +-
.../apache/sis/measure/LinearConverterTest.java | 14 +--
7 files changed, 151 insertions(+), 33 deletions(-)
diff --git
a/core/sis-utility/src/main/java/org/apache/sis/measure/AbstractConverter.java
b/core/sis-utility/src/main/java/org/apache/sis/measure/AbstractConverter.java
index 4649573..a85e44d 100644
---
a/core/sis-utility/src/main/java/org/apache/sis/measure/AbstractConverter.java
+++
b/core/sis-utility/src/main/java/org/apache/sis/measure/AbstractConverter.java
@@ -140,7 +140,7 @@ abstract class AbstractConverter implements UnitConverter,
Serializable {
public UnitConverter concatenate(final UnitConverter converter) {
ArgumentChecks.ensureNonNull("converter", converter);
if (equals(converter.inverse())) {
- return LinearConverter.IDENTITY;
+ return IdentityConverter.INSTANCE;
}
return new ConcatenatedConverter(converter, this);
}
diff --git
a/core/sis-utility/src/main/java/org/apache/sis/measure/ConcatenatedConverter.java
b/core/sis-utility/src/main/java/org/apache/sis/measure/ConcatenatedConverter.java
index 18b4340..1569206 100644
---
a/core/sis-utility/src/main/java/org/apache/sis/measure/ConcatenatedConverter.java
+++
b/core/sis-utility/src/main/java/org/apache/sis/measure/ConcatenatedConverter.java
@@ -126,7 +126,7 @@ final class ConcatenatedConverter extends AbstractConverter
implements LenientCo
public UnitConverter concatenate(final UnitConverter converter) {
ArgumentChecks.ensureNonNull("converter", converter);
if (equals(converter.inverse())) {
- return LinearConverter.IDENTITY;
+ return IdentityConverter.INSTANCE;
}
// Delegate to c1 and c2 because they may provide more intelligent
'concatenate' implementations.
return c2.concatenate(c1.concatenate(converter));
diff --git
a/core/sis-utility/src/main/java/org/apache/sis/measure/ConventionalUnit.java
b/core/sis-utility/src/main/java/org/apache/sis/measure/ConventionalUnit.java
index 603a4ba..20cc956 100644
---
a/core/sis-utility/src/main/java/org/apache/sis/measure/ConventionalUnit.java
+++
b/core/sis-utility/src/main/java/org/apache/sis/measure/ConventionalUnit.java
@@ -310,7 +310,7 @@ final class ConventionalUnit<Q extends Quantity<Q>> extends
AbstractUnit<Q> {
@Override
public UnitConverter getConverterTo(final Unit<Q> that) throws
UnconvertibleException {
if (that == this) {
- return LinearConverter.IDENTITY;
+ return IdentityConverter.INSTANCE;
}
ArgumentChecks.ensureNonNull("that", that);
UnitConverter c = toTarget;
@@ -340,7 +340,7 @@ final class ConventionalUnit<Q extends Quantity<Q>> extends
AbstractUnit<Q> {
@Override
public UnitConverter getConverterToAny(final Unit<?> that) throws
IncommensurableException {
if (that == this) {
- return LinearConverter.IDENTITY;
+ return IdentityConverter.INSTANCE;
}
ArgumentChecks.ensureNonNull("that", that);
UnitConverter c = toTarget;
diff --git
a/core/sis-utility/src/main/java/org/apache/sis/measure/IdentityConverter.java
b/core/sis-utility/src/main/java/org/apache/sis/measure/IdentityConverter.java
new file mode 100644
index 0000000..721342e
--- /dev/null
+++
b/core/sis-utility/src/main/java/org/apache/sis/measure/IdentityConverter.java
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sis.measure;
+
+import javax.measure.UnitConverter;
+import org.apache.sis.util.ArgumentChecks;
+import org.apache.sis.util.ComparisonMode;
+import org.apache.sis.util.LenientComparable;
+
+
+/**
+ * Linear converter with a scale factor of 1 and an offset of 0. We define a
class for this special case
+ * instead than using the more generic {@link LinearConverter} class because
we want to avoid performing
+ * any arithmetic operation in the {@link #convert(double)} method, in order
to preserve negative zero:
+ *
+ * {@preformat java
+ * convert(-0d) ≡ -0d
+ * }
+ *
+ * When the value is used in a map projection parameter, its sign can have
implications in the chain of
+ * concatenated transforms. The final result is numerically equivalent, but
intermediate steps may differ
+ * depending on the parameter sign.
+ *
+ * @author Martin Desruisseaux (Geomatys)
+ * @version 1.0
+ * @since 1.0
+ * @module
+ */
+final class IdentityConverter extends AbstractConverter implements
LenientComparable {
+ /**
+ * For cross-version compatibility.
+ */
+ private static final long serialVersionUID = 1230536100139464866L;
+
+ /**
+ * The identity linear converter.
+ */
+ static final IdentityConverter INSTANCE = new IdentityConverter();
+
+ /**
+ * For {@link #INSTANCE} only.
+ */
+ private IdentityConverter() {
+ }
+
+ /** Straight forward implementation. */
+ @Override public boolean isLinear() {return true;}
+ @Override public boolean isIdentity() {return true;}
+ @Override public UnitConverter inverse() {return this;}
+ @Override public double convert(double value) {return value;}
+ @Override public double derivative(double value) {return 1;}
+ @Override public UnitConverter concatenate(UnitConverter c) {return c;}
+ @Override public String toString() {return "y =
x";}
+
+ /**
+ * Returns the value unchanged, with a check against null values
+ * for consistency with {@link LinearConverter#convert(Number)}.
+ */
+ @Override
+ public Number convert(Number value) {
+ ArgumentChecks.ensureNonNull("value", value);
+ return value;
+ }
+
+ /**
+ * Returns a hash code value for this unit converter.
+ */
+ @Override
+ public int hashCode() {
+ return (int) serialVersionUID;
+ }
+
+ /**
+ * Compares this converter with the given object for equality. This method
may return {@code true}
+ * only if {@code Object} is an instance of {@link IdentityConverter} or
{@link LinearConverter}.
+ * We apply this restriction in order to be symmetric with those cases,
+ * i.e. {@code A.equals(B)} = {@code B.equals(A)}.
+ */
+ @Override
+ public boolean equals(final Object other) {
+ // See method javadoc for why we restrict to AbstractConverter instead
of UnitConverter.
+ return (other instanceof AbstractConverter) && ((AbstractConverter)
other).isIdentity();
+ }
+
+ /**
+ * Compares this converter with the given object for equality, optionally
ignoring rounding errors.
+ */
+ @Override
+ public boolean equals(final Object other, final ComparisonMode mode) {
+ if (mode.isApproximate() && other instanceof LinearConverter) {
+ return ((LinearConverter) other).almostIdentity();
+ }
+ return (other instanceof UnitConverter) && ((UnitConverter)
other).isIdentity();
+ }
+}
diff --git
a/core/sis-utility/src/main/java/org/apache/sis/measure/LinearConverter.java
b/core/sis-utility/src/main/java/org/apache/sis/measure/LinearConverter.java
index 75e7acd..dc81ec3 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/measure/LinearConverter.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/measure/LinearConverter.java
@@ -53,11 +53,6 @@ final class LinearConverter extends AbstractConverter
implements LenientComparab
private static final long serialVersionUID = -3759983642723729926L;
/**
- * The identity linear converter.
- */
- static final LinearConverter IDENTITY = new LinearConverter(1, 0, 1);
-
- /**
* The scale to apply for converting values, before division by {@link
#divisor}.
*/
private final double scale;
@@ -101,7 +96,7 @@ final class LinearConverter extends AbstractConverter
implements LenientComparab
* Creates a linear converter from the given scale and offset, which may
be {@link BigDecimal} instances.
* This is the implementation of public {@link Units#converter(Number,
Number)} method.
*/
- static LinearConverter create(final Number scale, final Number offset) {
+ static AbstractConverter create(final Number scale, final Number offset) {
final double numerator, divisor;
double shift = (offset != null) ? doubleValue(offset) : 0;
if (scale instanceof Fraction) {
@@ -112,21 +107,16 @@ final class LinearConverter extends AbstractConverter
implements LenientComparab
numerator = (scale != null) ? doubleValue(scale) : 1;
divisor = 1;
}
- final LinearConverter c = create(numerator, shift, divisor);
+ if (shift == 0 && numerator == divisor) {
+ return IdentityConverter.INSTANCE;
+ }
+ final LinearConverter c = new LinearConverter(numerator, shift,
divisor);
if (scale instanceof BigDecimal) c.scale10 = (BigDecimal) scale;
if (offset instanceof BigDecimal) c.offset10 = (BigDecimal) offset;
return c;
}
/**
- * Returns a linear converter for the given scale and offset.
- */
- private static LinearConverter create(final double scale, final double
offset, final double divisor) {
- if (offset == 0 && scale == divisor) return IDENTITY;
- return new LinearConverter(scale, offset, divisor);
- }
-
- /**
* Returns a linear converter for the given ratio. The scale factor is
specified as a ratio because
* the unit conversion factors are often defined with a value in base 10.
That value is considered
* exact by definition, but IEEE 754 has no exact representation of
decimal fraction digits.
@@ -171,7 +161,7 @@ final class LinearConverter extends AbstractConverter
implements LenientComparab
denominator = lc.divisor;
} else {
// Subtraction by convert(0) is a paranoiac safety.
- numerator = converter.convert(1.0) - converter.convert(0.0);
+ numerator = converter.convert(1d) - converter.convert(0d);
denominator = 1;
}
if (root) {
@@ -221,6 +211,13 @@ final class LinearConverter extends AbstractConverter
implements LenientComparab
}
/**
+ * Returns {@code true} if this converter is close to an identity
converter.
+ */
+ final boolean almostIdentity() {
+ return epsilonEquals(scale, divisor) && epsilonEquals(offset, 0);
+ }
+
+ /**
* Returns the inverse of this unit converter.
* Given that the formula applied by this converter is:
*
@@ -248,7 +245,7 @@ final class LinearConverter extends AbstractConverter
implements LenientComparab
*/
@Override
@SuppressWarnings("fallthrough")
- Number[] coefficients() {
+ final Number[] coefficients() {
final Number[] c = new Number[(scale != divisor) ? 2 : (offset != 0) ?
1 : 0];
switch (c.length) {
case 2: c[1] = ratio(scale, divisor);
@@ -385,7 +382,10 @@ final class LinearConverter extends AbstractConverter
implements LenientComparab
otherOffset /= cf;
otherDivisor /= cf;
}
- return create(otherScale, otherOffset, otherDivisor);
+ if (otherOffset == 0 && otherScale == otherDivisor) {
+ return IdentityConverter.INSTANCE;
+ }
+ return new LinearConverter(otherScale, otherOffset, otherDivisor);
}
/**
@@ -409,6 +409,9 @@ final class LinearConverter extends AbstractConverter
implements LenientComparab
Numerics.equals(offset, o.offset) &&
Numerics.equals(divisor, o.divisor);
}
+ if (other instanceof IdentityConverter) {
+ return isIdentity();
+ }
return false;
}
@@ -418,7 +421,13 @@ final class LinearConverter extends AbstractConverter
implements LenientComparab
@Override
public boolean equals(final Object other, final ComparisonMode mode) {
if (mode.isApproximate()) {
- return (other instanceof LinearConverter) &&
equivalent((LinearConverter) other);
+ if (other instanceof LinearConverter) {
+ return equivalent((LinearConverter) other);
+ } else if (other instanceof IdentityConverter) {
+ return almostIdentity();
+ } else {
+ return false;
+ }
} else {
return equals(other);
}
@@ -428,7 +437,7 @@ final class LinearConverter extends AbstractConverter
implements LenientComparab
* Returns {@code true} if the given converter perform the same conversion
than this converter,
* except for rounding errors.
*/
- boolean equivalent(final LinearConverter other) {
+ final boolean equivalent(final LinearConverter other) {
return epsilonEquals(scale * other.divisor, other.scale * divisor) &&
epsilonEquals(offset * other.divisor, other.offset * divisor);
}
diff --git
a/core/sis-utility/src/main/java/org/apache/sis/measure/SystemUnit.java
b/core/sis-utility/src/main/java/org/apache/sis/measure/SystemUnit.java
index d7caac0..fdaa50d 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/measure/SystemUnit.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/measure/SystemUnit.java
@@ -359,7 +359,7 @@ final class SystemUnit<Q extends Quantity<Q>> extends
AbstractUnit<Q> implements
throw new UnconvertibleException(incompatible(unit));
}
if (step == unit) {
- return LinearConverter.IDENTITY;
+ return IdentityConverter.INSTANCE;
}
/*
* At this point we know that the given units is not a system unit.
Ask the conversion
@@ -389,7 +389,7 @@ final class SystemUnit<Q extends Quantity<Q>> extends
AbstractUnit<Q> implements
throw new IncommensurableException(incompatible(unit));
}
if (step == unit) {
- return LinearConverter.IDENTITY;
+ return IdentityConverter.INSTANCE;
}
// Same remark than in getConverterTo(Unit).
return unit.getConverterToAny(step).inverse();
diff --git
a/core/sis-utility/src/test/java/org/apache/sis/measure/LinearConverterTest.java
b/core/sis-utility/src/test/java/org/apache/sis/measure/LinearConverterTest.java
index c4b3ca0..d8138ce 100644
---
a/core/sis-utility/src/test/java/org/apache/sis/measure/LinearConverterTest.java
+++
b/core/sis-utility/src/test/java/org/apache/sis/measure/LinearConverterTest.java
@@ -43,7 +43,7 @@ public final strictfp class LinearConverterTest extends
TestCase {
* @param denominator the expected denominator in the conversion factor.
* @param converter the converter to verify.
*/
- static void assertScale(final int numerator, final int denominator, final
LinearConverter converter) {
+ static void assertScale(final int numerator, final int denominator, final
AbstractConverter converter) {
final double derivative = numerator / (double) denominator;
final Number[] coefficients = converter.coefficients();
assertEquals("coefficients.length", 2, coefficients.length);
@@ -88,7 +88,7 @@ public final strictfp class LinearConverterTest extends
TestCase {
*/
@Test
public void testIsIdentityAndLinear() {
- LinearConverter c = LinearConverter.IDENTITY;
+ AbstractConverter c = IdentityConverter.INSTANCE;
assertTrue(c.isIdentity());
assertTrue(c.isLinear());
assertEquals("coefficients.length", 0, c.coefficients().length);
@@ -185,15 +185,15 @@ public final strictfp class LinearConverterTest extends
TestCase {
*/
@Test
public void testConcatenate() {
- LinearConverter c = LinearConverter.scale(254, 100);
// inches to centimetres
+ AbstractConverter c = LinearConverter.scale(254, 100);
// inches to centimetres
assertScale(254, 100, c);
- c = (LinearConverter) c.concatenate(LinearConverter.scale(10, 1));
// centimetres to millimetres
+ c = (AbstractConverter) c.concatenate(LinearConverter.scale(10, 1));
// centimetres to millimetres
assertScale(254, 10, c);
- c = (LinearConverter) c.concatenate(LinearConverter.scale(1, 1000));
// millimetres to metres
+ c = (AbstractConverter) c.concatenate(LinearConverter.scale(1, 1000));
// millimetres to metres
assertScale(254, 10000, c);
c = LinearConverter.offset(27315, 100);
// Celsius to kelvin
- c = (LinearConverter) c.concatenate(LinearConverter.offset(-54630,
200));
+ c = (AbstractConverter) c.concatenate(LinearConverter.offset(-54630,
200));
assertTrue(c.isIdentity());
}
@@ -228,7 +228,7 @@ public final strictfp class LinearConverterTest extends
TestCase {
*/
@Test
public void testToString() {
- assertEquals("y = x", LinearConverter.IDENTITY
.toString());
+ assertEquals("y = x", IdentityConverter.INSTANCE
.toString());
assertEquals("y = 100⋅x", LinearConverter.scale ( 100,
1).toString());
assertEquals("y = x∕100", LinearConverter.scale ( 1,
100).toString());
assertEquals("y = 254⋅x∕100", LinearConverter.scale ( 254,
100).toString());
