From e890e4044252abbcbee7a2da7df8c68f08c8bbce Mon Sep 17 00:00:00 2001 From: Mikhail Bautin Date: Mon, 11 Jan 2016 17:16:07 -0800 Subject: [PATCH 1/5] Use HALF_EVEN rounding for the double type in generated code --- .../expressions/mathExpressions.scala | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala index 604c52713e972..123ae5b3081e0 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala @@ -764,12 +764,42 @@ case class Round(child: Expression, scale: Expression) }""" } case DoubleType => // if child eval to NaN or Infinity, just return it. + // The logic for rounding half-integers to even values is exemplified by the following + // table: + // + // x | x rounded to half-even | x * 2 | (x rounded to half-even) * 2 | (x * 2) & 3 + // ---------------------------------------------------------------------------------------- + // -4.5 | -4 | -9 | -8 | 3 + // -3.5 | -4 | -7 | -8 | 1 + // -2.5 | -2 | -5 | -6 | 3 + // -1.5 | -2 | -3 | -6 | 1 + // -0.5 | 0 | -1 | 0 | 3 + // 0.5 | 0 | 1 | 0 | 1 + // 1.5 | 2 | 3 | 4 | 3 + // 2.5 | 2 | 5 | 4 | 1 + // 3.5 | 4 | 7 | 8 | 3 + // 4.5 | 4 | 9 | 8 | 1 + // + // Therefore, looking at the last three columns above, if x has the form of ".5", + // then + // (x rounded to half-even) * 2 = (x * 2) + ((x * 2) & 3) - 2 + if (_scale == 0) { s""" if (Double.isNaN(${ce.primitive}) || Double.isInfinite(${ce.primitive})){ ${ev.primitive} = ${ce.primitive}; } else { - ${ev.primitive} = Math.round(${ce.primitive}); + double timesTwo = ${ce.primitive} * 2; + long timesTwoRounded = Math.round(timesTwo); + if (timesTwo == timesTwoRounded) { + if ((timesTwo & 1) == 0) { + ${ev.primitive} = timesTwo >> 1; + } else { + ${ev.primitive} = (timesTwo + (timesTwoRounded & 3) - 2) >> 1; + } + } else { + ${ev.primitive} = Math.round(${ce.primitive}); + } }""" } else { s""" From 2b85dcb2017d2b86fbea486124118ef1936459a0 Mon Sep 17 00:00:00 2001 From: Mikhail Bautin Date: Mon, 11 Jan 2016 17:26:12 -0800 Subject: [PATCH 2/5] Do not attempt to apply the "&" operator to a double --- .../apache/spark/sql/catalyst/expressions/mathExpressions.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala index 123ae5b3081e0..4973fe52384b6 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala @@ -792,7 +792,7 @@ case class Round(child: Expression, scale: Expression) double timesTwo = ${ce.primitive} * 2; long timesTwoRounded = Math.round(timesTwo); if (timesTwo == timesTwoRounded) { - if ((timesTwo & 1) == 0) { + if ((timesTwoRounded & 1) == 0) { ${ev.primitive} = timesTwo >> 1; } else { ${ev.primitive} = (timesTwo + (timesTwoRounded & 3) - 2) >> 1; From 0c6f68999862b4b2b1afc91a75471f399fb4f55f Mon Sep 17 00:00:00 2001 From: Mikhail Bautin Date: Mon, 11 Jan 2016 17:41:15 -0800 Subject: [PATCH 3/5] Do not apply the bit shift operation to double --- .../apache/spark/sql/catalyst/expressions/mathExpressions.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala index 4973fe52384b6..cb11953b00f20 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala @@ -793,7 +793,7 @@ case class Round(child: Expression, scale: Expression) long timesTwoRounded = Math.round(timesTwo); if (timesTwo == timesTwoRounded) { if ((timesTwoRounded & 1) == 0) { - ${ev.primitive} = timesTwo >> 1; + ${ev.primitive} = timesTwoRounded >> 1; } else { ${ev.primitive} = (timesTwo + (timesTwoRounded & 3) - 2) >> 1; } From e5269fffa2390800348a0cd2d2b578c0fc97e417 Mon Sep 17 00:00:00 2001 From: Mikhail Bautin Date: Mon, 11 Jan 2016 18:02:16 -0800 Subject: [PATCH 4/5] Do not use the shift operator on a double in another place --- .../apache/spark/sql/catalyst/expressions/mathExpressions.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala index cb11953b00f20..4490a86a762f8 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala @@ -795,7 +795,7 @@ case class Round(child: Expression, scale: Expression) if ((timesTwoRounded & 1) == 0) { ${ev.primitive} = timesTwoRounded >> 1; } else { - ${ev.primitive} = (timesTwo + (timesTwoRounded & 3) - 2) >> 1; + ${ev.primitive} = (timesTwoRounded + (timesTwoRounded & 3) - 2) >> 1; } } else { ${ev.primitive} = Math.round(${ce.primitive}); From b64ae1e6be230795e7b65fe7561f2303d4387804 Mon Sep 17 00:00:00 2001 From: Mikhail Bautin Date: Tue, 12 Jan 2016 13:22:50 -0800 Subject: [PATCH 5/5] Fix MathFunctionsSuite with the updated rounding implementation (HALF_EVEN) --- .../spark/sql/catalyst/expressions/MathFunctionsSuite.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala index 90c59f240b542..45e728e8483ec 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala @@ -512,7 +512,7 @@ class MathFunctionsSuite extends SparkFunSuite with ExpressionEvalHelper { Seq.fill[Short](7)(31415) val intResults: Seq[Int] = Seq(314000000, 314200000, 314160000, 314159000, 314159300, - 314159270) ++ Seq.fill(7)(314159265) + 314159260) ++ Seq.fill(7)(314159265) val longResults: Seq[Long] = Seq(31415926536000000L, 31415926535900000L, 31415926535900000L, 31415926535898000L, 31415926535897900L, 31415926535897930L) ++