Repository: amzn/ion-java
Pull request #243 (2019-05-29 23:03:44)

Adds an Equivalence option to specify an epsilon to use when comparing float values.
*Description of changes:*

When experimenting with writing single-precision Ion floats, I realized that many such floats were not comparing equivalent after round trip. That's because `IonReader.doubleValue` extends the value to double-precision, which can result in a value that is slightly different than the original single-precision input. Specifying an epsilon to be used when comparing these values allows the user to opt in to ignoring this small difference.

To avoid telescoping static methods, I added a builder. To avoid ballooning method signatures, I added a configuration object (note: for the existing static methods, the configuration is constant). New options (e.g. giving "timestamp instant equals" its own option independent of "strict") should be added to the builder. Most applications will use only one, maybe two, equivalence configurations. These may be stored in constants and reused throughout the application. The existing static methods continue to work as before.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.


codecov[bot]: # [Codecov](https://codecov.io/gh/amzn/ion-java/pull/243?src=pr&el=h1) Report
> Merging [#243](https://codecov.io/gh/amzn/ion-java/pull/243?src=pr&el=desc) into [master](https://codecov.io/gh/amzn/ion-java/commit/a2eb014633ae90addff92c73d3d29bfb3ada75e5?src=pr&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `90.9%`.

[![Impacted file tree graph](https://codecov.io/gh/amzn/ion-java/pull/243/graphs/tree.svg?width=650&token=M93Y3e3kKn&height=150&src=pr)](https://codecov.io/gh/amzn/ion-java/pull/243?src=pr&el=tree)

```diff
@@             Coverage Diff              @@
##             master     #243      +/-   ##
============================================
+ Coverage     63.78%   63.82%   +0.03%     
- Complexity     4794     4799       +5     
============================================
  Files           136      136              
  Lines         20952    20974      +22     
  Branches       3818     3820       +2     
============================================
+ Hits          13365    13387      +22     
+ Misses         6246     6245       -1     
- Partials       1341     1342       +1
```


| [Impacted Files](https://codecov.io/gh/amzn/ion-java/pull/243?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [src/com/amazon/ion/util/Equivalence.java](https://codecov.io/gh/amzn/ion-java/pull/243/diff?src=pr&el=tree#diff-c3JjL2NvbS9hbWF6b24vaW9uL3V0aWwvRXF1aXZhbGVuY2UuamF2YQ==) | `90.5% <90.9%> (+0.06%)` | `60 <7> (+4)` | :arrow_up: |
| [src/com/amazon/ion/impl/BlockedBuffer.java](https://codecov.io/gh/amzn/ion-java/pull/243/diff?src=pr&el=tree#diff-c3JjL2NvbS9hbWF6b24vaW9uL2ltcGwvQmxvY2tlZEJ1ZmZlci5qYXZh) | `51.45% <0%> (+0.24%)` | `79% <0%> (+1%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/amzn/ion-java/pull/243?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/amzn/ion-java/pull/243?src=pr&el=footer). Last update [a2eb014...14f73b3](https://codecov.io/gh/amzn/ion-java/pull/243?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/com/amazon/ion/util/Equivalence.java b/src/com/amazon/ion/util/Equivalence.java
index dbbd391b..70759b88 100644
--- a/src/com/amazon/ion/util/Equivalence.java
+++ b/src/com/amazon/ion/util/Equivalence.java
@@ -61,6 +61,20 @@ import java.util.Set;
  *</pre>
  *
  * <p>
+ * Additional options are available by configuring an <code>Equivalence</code>
+ * instance using {@link Equivalence.Builder}. For example:
+ *
+ * <pre>
+ *     com.amazon.ion.util.Equivalence equivalence =
+ *         new com.amazon.ion.util.Equivalence.Builder()
+ *             .withEpsilon(1e-6)
+ *             .build();
+ *     IonValue v1 = ...;
+ *     IonValue v2 = ...;
+ *     equivalence.ionValueEquals( v1, v2 );
+ * </pre>
+ *
+ * <p>
  * <h3>Ion Equivalence</h3>
  * In order to make Ion a useful model to describe data, we must first define
  * the notion of equivalence for all values representable in Ion. Equivalence
@@ -104,7 +118,19 @@ import java.util.Set;
  * (A, V), where A is an ordered list of annotations, and V is an Ion Primitive
  * Data or Ion Complex Data value. The list of annotations, A is an tuple of Ion
  * Symbols (a specific type of Ion Primitive).
-
+ *
+ * <p>
+ * <h3>Terminology</h3>
+ * Within this class, <i>strict</i> equivalence refers to Ion data model
+ * equivalence as defined above and by the
+ * <a href="http://amzn.github.io/ion-docs/docs/spec.html">Ion
+ * Specification</a>. <i>Structural</i> or <i>non-strict</i> equivalence
+ * follows the same rules as strict equivalence, except that
+ * <ul>
+ *   <li>Annotations are not considered, and</li>
+ *   <li>Timestamps that represent the same instant in time are always
+ *   considered equivalent.</li>
+ * </ul>
  */
 public final class Equivalence {
 
@@ -115,7 +141,78 @@ public final class Equivalence {
      */
     private static final boolean PUBLIC_COMPARISON_API = false;
 
-    private Equivalence() {
+    /**
+     * Configuration that requires strict data equivalence.
+     * @see #ionEquals(IonValue, IonValue)
+     */
+    private static final Configuration STRICT_CONFIGURATION = new Configuration(new Builder().withStrict(true));
+
+    /**
+     * Configuration that requires structural equivalence without considering annotations.
+     * @see #ionEqualsByContent(IonValue, IonValue)
+     */
+    private static final Configuration NON_STRICT_CONFIGURATION = new Configuration(new Builder().withStrict(false));
+
+    /**
+     * Contains the configuration to use when comparing Ion values.
+     */
+    static final class Configuration {
+        private final boolean isStrict;
+        private final Double epsilon;
+
+        Configuration(Builder builder) {
+            this.isStrict = builder.isStrict;
+            this.epsilon = builder.epsilon;
+        }
+    }
+
+    /**
+     * Constructs {@link Equivalence} instances.
+     */
+    public static final class Builder {
+
+        private boolean isStrict = true;
+        private Double epsilon = null;
+
+        /**
+         * When true, checks for strict data equivalence over two Ion Values.
+         * When false, checks for structural data equivalence over two Ion
+         * Values. See {@link Equivalence} for discussion of the differences
+         * between the two.
+         * Default: true.
+         * @param isStrict the value.
+         * @return this builder.
+         */
+        public Builder withStrict(boolean isStrict) {
+            this.isStrict = isStrict;
+            return this;
+        }
+
+        /**
+         * The maximum absolute difference between two Ion float values for
+         * which the two values will be considered equivalent. Default: Ion
+         * float values will only be considered equivalent when
+         * <code>Double.compare(a, b) == 0</code>.
+         * @param epsilon the value.
+         * @return this builder.
+         */
+        public Builder withEpsilon(double epsilon) {
+            this.epsilon = epsilon;
+            return this;
+        }
+
+        /**
+         * @return a new Equivalence using this builder's configuration.
+         */
+        public Equivalence build() {
+            return new Equivalence(new Configuration(this));
+        }
+    }
+
+    private final Configuration configuration;
+
+    private Equivalence(Configuration configuration) {
+        this.configuration = configuration;
     }
 
 
@@ -166,13 +263,13 @@ public final class Equivalence {
      * and cannot contain duplicate elements, hence we cannot use it.
      */
     private static final Map<Field, Field>
-        convertToMultiSet(final IonStruct struct, final boolean strict) {
+        convertToMultiSet(final IonStruct struct, final Configuration configuration) {
 
         final Map<Field, Field> structMultiSet =
             new HashMap<Field, Field>();
 
         for (final IonValue val : struct) {
-            final Field item = new Field(val, strict);
+            final Field item = new Field(val, configuration);
             Field curr = structMultiSet.put(item, item);
             // curr will be non-null if the multi-set already contains the
             // name/value pair
@@ -193,7 +290,7 @@ public final class Equivalence {
 
     private static int compareStructs(final IonStruct s1,
                                       final IonStruct s2,
-                                      boolean strict)
+                                      final Configuration configuration)
     {
         int result = s1.size() - s2.size();
         if (result == 0) {
@@ -201,7 +298,7 @@ public final class Equivalence {
             // Map<Field, Field>). Refer to convertToMultiSet()'s
             // documentation for more info
             final Map<Field, Field> s1MultiSet
-                    = convertToMultiSet(s1, strict);
+                    = convertToMultiSet(s1, configuration);
 
             // Iterates through each name/value pair in IonStruct s2 and
             // determine if it also occurs in s1MultiSet.
@@ -209,7 +306,7 @@ public final class Equivalence {
             //          If it does, remove an occurrence from s1MultiSet
             //          If it doesn't, the two IonStructs aren't equal
             for (IonValue val : s2) {
-                Field field = new Field(val, strict);
+                Field field = new Field(val, configuration);
 
                 // Find an occurrence of the name/value pair in s1MultiSet
                 Field mappedValue = s1MultiSet.get(field);
@@ -231,7 +328,7 @@ public final class Equivalence {
 
     private static int compareSequences(final IonSequence s1,
                                         final IonSequence s2,
-                                        boolean strict)
+                                        final Configuration configuration)
     {
         int result = s1.size() - s2.size();
         if (result == 0) {
@@ -240,7 +337,7 @@ public final class Equivalence {
             while (iter1.hasNext()) {
                 result = ionCompareToImpl(iter1.next(),
                                           iter2.next(),
-                                          strict);
+                                          configuration);
                 if (result != 0) {
                     break;
                 }
@@ -315,7 +412,7 @@ public final class Equivalence {
      * a single {@code Field} -> {@code Field} with {@code occurrences} of 2.
      * <p>
      * Refer to
-     * {@link Equivalence#convertToMultiSet(IonStruct, boolean)} and
+     * {@link Equivalence#convertToMultiSet(IonStruct, Configuration)} and
      * {@link Field#equals(Object)} for more info.
      * <p>
      * NOTE: This class should only be instantiated for the sole purpose of
@@ -324,7 +421,7 @@ public final class Equivalence {
     static class Field {
         private final String    name; // aka field name
         private final IonValue  value;
-        private final boolean   strict;
+        private final Configuration configuration;
 
         /**
          * Number of times that this specific field (with the same name
@@ -332,7 +429,7 @@ public final class Equivalence {
          */
         private int occurrences;
 
-        Field(final IonValue value, final boolean strict)
+        Field(final IonValue value, final Configuration configuration)
         {
             SymbolToken tok = value.getFieldNameSymbol();
             String name = tok.getText();
@@ -342,7 +439,7 @@ public final class Equivalence {
             }
             this.name = name;
             this.value = value;
-            this.strict = strict;
+            this.configuration = configuration;
 
             // Occurrences of this name/value pair is 0 initially
             this.occurrences = 0;
@@ -371,20 +468,20 @@ public final class Equivalence {
             final Field sOther = (Field) other;
 
             return name.equals(sOther.name)
-                && ionEqualsImpl(value, ((Field) other).value, strict);
+                && ionEqualsImpl(value, ((Field) other).value, configuration);
         }
     }
 
     private static boolean ionEqualsImpl(final IonValue v1,
-                                               final IonValue v2,
-                                               final boolean strict)
+                                         final IonValue v2,
+                                         final Configuration configuration)
     {
-        return (ionCompareToImpl(v1, v2, strict) == 0);
+        return (ionCompareToImpl(v1, v2, configuration) == 0);
     }
 
     private static int ionCompareToImpl(final IonValue v1,
                                         final IonValue v2,
-                                        final boolean strict)
+                                        final Configuration configuration)
     {
         int result = 0;
 
@@ -431,8 +528,14 @@ public final class Equivalence {
                              ((IonInt) v2).bigIntegerValue());
                     break;
                 case FLOAT:
-                    result = Double.compare(((IonFloat) v1).doubleValue(),
-                                            ((IonFloat) v2).doubleValue());
+                    double double1 = ((IonFloat) v1).doubleValue();
+                    double double2 = ((IonFloat) v2).doubleValue();
+                    if (configuration.epsilon != null
+                            && (double1 == double2 || Math.abs(double1 - double2) <= configuration.epsilon)) {
+                        result = 0;
+                    } else {
+                        result = Double.compare(double1, double2);
+                    }
                     break;
                 case DECIMAL:
                     assert !PUBLIC_COMPARISON_API; // TODO amzn/ion-java/issues/26
@@ -441,7 +544,7 @@ public final class Equivalence {
                                             ? 0 : 1;
                     break;
                 case TIMESTAMP:
-                    if (strict) {
+                    if (configuration.isStrict) {
                         assert !PUBLIC_COMPARISON_API; // TODO amzn/ion-java/issues/26
                         result = (((IonTimestamp) v1).timestampValue().equals(
                                   ((IonTimestamp) v2).timestampValue())
@@ -473,14 +576,14 @@ public final class Equivalence {
                     assert !PUBLIC_COMPARISON_API; // TODO amzn/ion-java/issues/26
                     result = compareStructs((IonStruct) v1,
                                             (IonStruct) v2,
-                                            strict);
+                                            configuration);
                     break;
                 case LIST:
                 case SEXP:
                 case DATAGRAM:
                     result = compareSequences((IonSequence) v1,
                                               (IonSequence) v2,
-                                              strict);
+                                              configuration);
                     break;
                 }
             }
@@ -488,7 +591,7 @@ public final class Equivalence {
 
         // if the values are otherwise equal, but the caller wants strict
         // comparison, then we check the annotations
-        if ((result == 0) && strict) {
+        if ((result == 0) && configuration.isStrict) {
             // check tuple equality over the annotations
             // (which are symbol tokens)
             result = compareAnnotations(v1.getTypeAnnotationSymbols(),
@@ -511,7 +614,7 @@ public final class Equivalence {
     public static boolean ionEquals(final IonValue v1,
                                     final IonValue v2)
     {
-        return ionEqualsImpl(v1, v2, true);
+        return ionEqualsImpl(v1, v2, STRICT_CONFIGURATION);
     }
 
     /**
@@ -529,7 +632,24 @@ public final class Equivalence {
     public static boolean ionEqualsByContent(final IonValue v1,
                                              final IonValue v2)
     {
-        return ionEqualsImpl(v1, v2, false);
+        return ionEqualsImpl(v1, v2, NON_STRICT_CONFIGURATION);
+    }
+
+    /**
+     * Checks for data equivalence over two Ion values using this Equivalence's
+     * configuration
+     *
+     * @see Builder
+     *
+     * @param v1
+     *            The first Ion value to compare.
+     * @param v2
+     *            The second Ion value to compare.
+     *
+     * @return true if two Ion Values represent the same data.
+     */
+    public boolean ionValueEquals(final IonValue v1, final IonValue v2) {
+        return ionEqualsImpl(v1, v2, configuration);
     }
 
 }


Test patch:
diff --git a/test/com/amazon/ion/util/EquivalenceTest.java b/test/com/amazon/ion/util/EquivalenceTest.java
index 67646918..8a552ec1 100644
--- a/test/com/amazon/ion/util/EquivalenceTest.java
+++ b/test/com/amazon/ion/util/EquivalenceTest.java
@@ -16,6 +16,7 @@
 package com.amazon.ion.util;
 
 import com.amazon.ion.IonFloat;
+import com.amazon.ion.IonList;
 import com.amazon.ion.IonStruct;
 import com.amazon.ion.IonTestCase;
 import com.amazon.ion.IonValue;
@@ -28,6 +29,9 @@ public class EquivalenceTest
     private static void assertIonEq(final IonValue left, final IonValue right) {
         assertTrue(Equivalence.ionEquals(left, right));
         assertTrue(Equivalence.ionEquals(right, left));
+        Equivalence equivalence = new Equivalence.Builder().build();
+        assertTrue(equivalence.ionValueEquals(left, right));
+        assertTrue(equivalence.ionValueEquals(right, left));
 
         // Redundancy check included here, in the case that IonValue#equals()
         // doesn't use Equivalence's implementation anymore.
@@ -40,6 +44,9 @@ public class EquivalenceTest
     private static void assertNotIonEq(final IonValue left, final IonValue right) {
         assertFalse(Equivalence.ionEquals(left, right));
         assertFalse(Equivalence.ionEquals(right, left));
+        Equivalence equivalence = new Equivalence.Builder().build();
+        assertFalse(equivalence.ionValueEquals(left, right));
+        assertFalse(equivalence.ionValueEquals(right, left));
 
         // Redundancy check included here, in the case that IonValue#equals()
         // doesn't use Equivalence's implementation anymore.
@@ -52,6 +59,9 @@ public class EquivalenceTest
     private static void assertIonEqForm(final IonValue left, final IonValue right) {
         assertTrue(Equivalence.ionEqualsByContent(left, right));
         assertTrue(Equivalence.ionEqualsByContent(right, left));
+        Equivalence equivalence = new Equivalence.Builder().withStrict(false).build();
+        assertTrue(equivalence.ionValueEquals(left, right));
+        assertTrue(equivalence.ionValueEquals(right, left));
     }
 
     private IonValue ion(final String raw) {
@@ -375,9 +385,10 @@ public class EquivalenceTest
 
         assertEquals(3, struct.size());
 
-        Equivalence.Field f1 = new Equivalence.Field(v1, true);
-        Equivalence.Field f2 = new Equivalence.Field(v2, true);
-        Equivalence.Field f3 = new Equivalence.Field(v3, true);
+        Equivalence.Configuration configuration = new Equivalence.Configuration(new Equivalence.Builder().withStrict(true));
+        Equivalence.Field f1 = new Equivalence.Field(v1, configuration);
+        Equivalence.Field f2 = new Equivalence.Field(v2, configuration);
+        Equivalence.Field f3 = new Equivalence.Field(v3, configuration);
 
         assertFalse(f1.equals(f2));
         assertFalse(f1.equals(f3));
@@ -407,9 +418,10 @@ public class EquivalenceTest
 
         assertEquals(3, struct.size());
 
-        Equivalence.Field f1 = new Equivalence.Field(v1, true);
-        Equivalence.Field f2 = new Equivalence.Field(v2, true);
-        Equivalence.Field f3 = new Equivalence.Field(v3, true);
+        Equivalence.Configuration configuration = new Equivalence.Configuration(new Equivalence.Builder().withStrict(true));
+        Equivalence.Field f1 = new Equivalence.Field(v1, configuration);
+        Equivalence.Field f2 = new Equivalence.Field(v2, configuration);
+        Equivalence.Field f3 = new Equivalence.Field(v3, configuration);
 
         assertEquals(f1, f2);
         assertEquals(f2, f1); // symmetric
@@ -420,4 +432,42 @@ public class EquivalenceTest
         assertEquals(f2, f3); // transitive
         assertEquals(f3, f2); // symmetric
     }
+
+    @Test
+    public void builderWithoutEpsilon() {
+        Equivalence equivalence = new Equivalence.Builder().build();
+        IonFloat v1 = ionFloat(3.14);
+        IonFloat v2 = ionFloat(3.14 + 1e-7);
+        assertFalse(equivalence.ionValueEquals(v1, v2));
+        assertFalse(equivalence.ionValueEquals(v2, v1));
+        IonStruct struct1 = system().newEmptyStruct();
+        struct1.add("foo", v1.clone());
+        IonStruct struct2 = system().newEmptyStruct();
+        struct2.add("foo", v2.clone());
+        assertFalse(equivalence.ionValueEquals(struct1, struct2));
+        assertFalse(equivalence.ionValueEquals(struct2, struct1));
+        IonList list1 = system().newList(v1.clone());
+        IonList list2 = system().newList(v2.clone());
+        assertFalse(equivalence.ionValueEquals(list1, list2));
+        assertFalse(equivalence.ionValueEquals(list2, list1));
+    }
+
+    @Test
+    public void builderWithEpsilon() {
+        Equivalence equivalence = new Equivalence.Builder().withEpsilon(1e-6).build();
+        IonFloat v1 = ionFloat(3.14);
+        IonFloat v2 = ionFloat(3.14 + 1e-7);
+        assertTrue(equivalence.ionValueEquals(v1, v2));
+        assertTrue(equivalence.ionValueEquals(v2, v1));
+        IonStruct struct1 = system().newEmptyStruct();
+        struct1.add("foo", v1.clone());
+        IonStruct struct2 = system().newEmptyStruct();
+        struct2.add("foo", v2.clone());
+        assertTrue(equivalence.ionValueEquals(struct1, struct2));
+        assertTrue(equivalence.ionValueEquals(struct2, struct1));
+        IonList list1 = system().newList(v1.clone());
+        IonList list2 = system().newList(v2.clone());
+        assertTrue(equivalence.ionValueEquals(list1, list2));
+        assertTrue(equivalence.ionValueEquals(list2, list1));
+    }
 }