Skip to content

Commit 3b1d078

Browse files
xiaoxuandevpeter-toth
authored andcommitted
[SPARK-56089][SQL] Align asinh/acosh with fdlibm algorithm for cross-engine compatibility
### What changes were proposed in this pull request? Replace the naive `log(x + sqrt(x^2 ± 1))` formula in `Asinh` and `Acosh` with the full fdlibm algorithm (`s_asinh.c` / `e_acosh.c`), as implemented in OpenJDK `FdLibm.java`. For `Acosh` (5 branches): - `x < 1`: NaN - `x >= 2^28`: `log(x) + log(2)` - `x == 1`: `0.0` - `2 < x < 2^28`: `log(2x - 1/(x + sqrt(x*x - 1)))` - `1 < x < 2`: `log1p(t + sqrt(2*t + t*t))` For `Asinh` (4 branches): - `|x| < 2^-28`: `x` (identity) - `|x| > 2^28`: `sign(x) * (log(|x|) + log(2))` - `|x| > 2`: `sign(x) * log(2|x| + 1/(|x| + sqrt(x*x + 1)))` - otherwise: `sign(x) * log1p(|x| + x*x/(1 + sqrt(1 + x*x)))` ### Why are the changes needed? The fdlibm algorithm (`s_asinh.c` / `e_acosh.c`) is the de facto standard used by OpenJDK, glibc, musl, Go, Python, PostgreSQL, and DuckDB. Spark's previous naive formula `log(x + sqrt(x^2 ± 1))` produces results that differ by 1-2 ULP from these platforms for certain input ranges, causing cross-engine inconsistencies. The fdlibm algorithm also avoids catastrophic cancellation near x=1 (acosh) and x=0 (asinh) by using range-specific formulas (log1p, identity). This also changes the large-value threshold from `sqrt(Double.MaxValue)` to `2^28`, matching the fdlibm standard. ### Does this PR introduce _any_ user-facing change? Results may differ by 1-2 ULP for inputs in the `(1, 2]` range (acosh) and `[-2, 2]` range (asinh) due to the more precise formulas. The new results are closer to the mathematically exact values and consistent with other platforms. ### How was this patch tested? Updated tests in `MathExpressionsSuite` with fdlibm-aligned reference functions, plus new tests covering all branches with hardcoded reference values cross-verified against C libm. ### Was this patch authored or co-authored using generative AI tooling? Yes, co-authored with Kiro. Closes #54912 from xiaoxuandev/fix-56089. Authored-by: Xiaoxuan Li <xioxuan@amazon.com> Signed-off-by: Peter Toth <peter.toth@gmail.com>
1 parent dce5047 commit 3b1d078

4 files changed

Lines changed: 142 additions & 32 deletions

File tree

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala

Lines changed: 58 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -418,25 +418,36 @@ case class Cosh(child: Expression) extends UnaryMathExpression(math.cosh, "COSH"
418418
since = "3.0.0",
419419
group = "math_funcs")
420420
case class Acosh(child: Expression)
421-
extends UnaryMathExpression((x: Double) => x match {
422-
// in case of large values, the square would lead to Infinity; also, - 1 would be ignored due
423-
// to numeric precision. So log(x + sqrt(x * x - 1)) becomes log(2x) = log(2) + log(x) for
424-
// positive values.
425-
case x if x >= Math.sqrt(Double.MaxValue) =>
426-
StrictMath.log(2) + StrictMath.log(x)
427-
case x if x < 1 =>
421+
extends UnaryMathExpression((x: Double) => {
422+
// fdlibm e_acosh.c algorithm
423+
if (x < 1.0) {
428424
Double.NaN
429-
case _ => StrictMath.log(x + math.sqrt(x * x - 1.0)) }, "ACOSH") {
425+
} else if (x >= (1 << 28)) {
426+
StrictMath.log(x) + StrictMath.log(2.0)
427+
} else if (x == 1.0) {
428+
0.0
429+
} else if (x > 2.0) {
430+
StrictMath.log(2.0 * x - 1.0 / (x + math.sqrt(x * x - 1.0)))
431+
} else {
432+
val t = x - 1.0
433+
StrictMath.log1p(t + math.sqrt(2.0 * t + t * t))
434+
}
435+
}, "ACOSH") {
430436
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
431437
nullSafeCodeGen(ctx, ev, c => {
432438
val sm = "java.lang.StrictMath"
433439
s"""
434-
|if ($c >= ${Math.sqrt(Double.MaxValue)}) {
435-
| ${ev.value} = $sm.log($c) + $sm.log(2);
436-
|} else if ($c < 1) {
440+
|if ($c < 1.0) {
437441
| ${ev.value} = java.lang.Double.NaN;
442+
|} else if ($c >= ${1 << 28}.0) {
443+
| ${ev.value} = $sm.log($c) + $sm.log(2.0);
444+
|} else if ($c == 1.0) {
445+
| ${ev.value} = 0.0;
446+
|} else if ($c > 2.0) {
447+
| ${ev.value} = $sm.log(2.0 * $c - 1.0 / ($c + java.lang.Math.sqrt($c * $c - 1.0)));
438448
|} else {
439-
| ${ev.value} = $sm.log($c + java.lang.Math.sqrt($c * $c - 1.0));
449+
| double t = $c - 1.0;
450+
| ${ev.value} = $sm.log1p(t + java.lang.Math.sqrt(2.0 * t + t * t));
440451
|}
441452
|""".stripMargin
442453
})
@@ -865,20 +876,43 @@ case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH"
865876
since = "3.0.0",
866877
group = "math_funcs")
867878
case class Asinh(child: Expression)
868-
extends UnaryMathExpression((x: Double) => x match {
869-
// in case of large values, the square would lead to Infinity; also, + 1 would be ignored due
870-
// to numeric precision. So log(x + sqrt(x * x + 1)) becomes log(2x) = log(2) + log(x) for
871-
// positive values. Since the function is symmetric, for large values we can use
872-
// signum(x) + log(2|x|)
873-
case x if Math.abs(x) >= Math.sqrt(Double.MaxValue) - 1 =>
874-
Math.signum(x) * (StrictMath.log(2) + StrictMath.log(Math.abs(x)))
875-
case _ => StrictMath.log(x + math.sqrt(x * x + 1.0)) }, "ASINH") {
879+
extends UnaryMathExpression((x: Double) => {
880+
// fdlibm s_asinh.c algorithm
881+
val ax = Math.abs(x)
882+
val w = if (ax.isInfinite || ax.isNaN) {
883+
ax
884+
} else if (ax < 1.0 / (1 << 28)) {
885+
ax
886+
} else if (ax > (1 << 28)) {
887+
StrictMath.log(ax) + StrictMath.log(2.0)
888+
} else if (ax > 2.0) {
889+
StrictMath.log(2.0 * ax + 1.0 / (math.sqrt(x * x + 1.0) + ax))
890+
} else {
891+
val t = x * x
892+
StrictMath.log1p(ax + t / (1.0 + math.sqrt(1.0 + t)))
893+
}
894+
Math.copySign(w, x)
895+
}, "ASINH") {
876896
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
877-
defineCodeGen(ctx, ev, c => {
897+
nullSafeCodeGen(ctx, ev, c => {
878898
val sm = "java.lang.StrictMath"
879-
s"$sm.abs($c) >= ${Math.sqrt(Double.MaxValue) - 1} ? " +
880-
s"$sm.signum($c) * ($sm.log($sm.abs($c)) + $sm.log(2)) :" +
881-
s"$sm.log($c + java.lang.Math.sqrt($c * $c + 1.0))"
899+
s"""
900+
|double ax = java.lang.Math.abs($c);
901+
|double w;
902+
|if (java.lang.Double.isInfinite(ax) || java.lang.Double.isNaN(ax)) {
903+
| w = ax;
904+
|} else if (ax < ${1.0 / (1 << 28)}) {
905+
| w = ax;
906+
|} else if (ax > ${1 << 28}.0) {
907+
| w = $sm.log(ax) + $sm.log(2.0);
908+
|} else if (ax > 2.0) {
909+
| w = $sm.log(2.0 * ax + 1.0 / (java.lang.Math.sqrt($c * $c + 1.0) + ax));
910+
|} else {
911+
| double t = $c * $c;
912+
| w = $sm.log1p(ax + t / (1.0 + java.lang.Math.sqrt(1.0 + t)));
913+
|}
914+
|${ev.value} = java.lang.Math.copySign(w, $c);
915+
|""".stripMargin
882916
})
883917
}
884918
override protected def withNewChildInternal(newChild: Expression): Asinh = copy(child = newChild)

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathExpressionsSuite.scala

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,23 @@ class MathExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
238238
}
239239

240240
test("asinh") {
241-
testUnary(Asinh, (x: Double) => StrictMath.log(x + math.sqrt(x * x + 1.0)))
241+
// fdlibm-aligned reference function
242+
def asinhRef(x: Double): Double = {
243+
val ax = Math.abs(x)
244+
val w = if (ax.isInfinite || ax.isNaN) {
245+
ax
246+
} else if (ax < 1.0 / (1 << 28)) {
247+
ax
248+
} else if (ax > (1 << 28)) {
249+
StrictMath.log(ax) + StrictMath.log(2.0)
250+
} else if (ax > 2.0) {
251+
StrictMath.log(2.0 * ax + 1.0 / (math.sqrt(x * x + 1.0) + ax))
252+
} else {
253+
StrictMath.log1p(ax + x * x / (1.0 + math.sqrt(1.0 + x * x)))
254+
}
255+
Math.copySign(w, x)
256+
}
257+
testUnary(Asinh, asinhRef)
242258
checkConsistencyBetweenInterpretedAndCodegen(Asinh, DoubleType)
243259

244260
checkEvaluation(Asinh(Double.NegativeInfinity), Double.NegativeInfinity)
@@ -280,10 +296,25 @@ class MathExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
280296
}
281297

282298
test("acosh") {
283-
def f: (Double) => Double = (x: Double) => StrictMath.log(x + math.sqrt(x * x - 1.0))
284-
testUnary(Acosh, f, (10 to 20).map(_ * 0.1))
285-
testUnary(Acosh, f, (-20 to 9).map(_ * 0.1), expectNaN = true)
286-
checkConsistencyBetweenInterpretedAndCodegen(Cosh, DoubleType)
299+
// fdlibm-aligned reference function
300+
def acoshRef(x: Double): Double = {
301+
if (x < 1.0) {
302+
Double.NaN
303+
} else if (x >= (1 << 28)) {
304+
StrictMath.log(x) + StrictMath.log(2.0)
305+
} else if (x == 1.0) {
306+
0.0
307+
} else if (x > 2.0) {
308+
val t = x * x
309+
StrictMath.log(2.0 * x - 1.0 / (x + math.sqrt(t - 1.0)))
310+
} else {
311+
val t = x - 1.0
312+
StrictMath.log1p(t + math.sqrt(2.0 * t + t * t))
313+
}
314+
}
315+
testUnary(Acosh, acoshRef, (10 to 20).map(_ * 0.1))
316+
testUnary(Acosh, acoshRef, (-20 to 9).map(_ * 0.1), expectNaN = true)
317+
checkConsistencyBetweenInterpretedAndCodegen(Acosh, DoubleType)
287318

288319
val nullLit = Literal.create(null, NullType)
289320
val doubleNullLit = Literal.create(null, DoubleType)
@@ -1025,4 +1056,35 @@ class MathExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
10251056
checkNaN(Acosh(-Math.sqrt(Double.MaxValue) + 2))
10261057
}
10271058

1059+
test("SPARK-56089: asinh/acosh fdlibm algorithm coverage") {
1060+
// asinh: hardcoded reference values cross-verified against C libm (glibc/musl fdlibm)
1061+
checkEvaluation(Asinh(Literal(0.5)), 0.48121182505960347, EmptyRow)
1062+
checkEvaluation(Asinh(Literal(1.0)), 0.881373587019543, EmptyRow)
1063+
checkEvaluation(Asinh(Literal(2.0)), 1.4436354751788103, EmptyRow)
1064+
checkEvaluation(Asinh(Literal(10.0)), 2.99822295029797, EmptyRow)
1065+
checkEvaluation(Asinh(Literal(1e8)), 19.11382792451231, EmptyRow)
1066+
// |x| < 2^-28 (identity branch)
1067+
checkEvaluation(Asinh(Literal(1.0e-10)), 1.0e-10, EmptyRow)
1068+
// |x| > 2^28 branch
1069+
val asinhExpected = Math.log(Double.MaxValue) + StrictMath.log(2.0)
1070+
checkEvaluation(Asinh(Literal(Double.MaxValue)), asinhExpected, EmptyRow)
1071+
checkEvaluation(Asinh(Literal(-Double.MaxValue)), -asinhExpected, EmptyRow)
1072+
// infinity
1073+
checkEvaluation(Asinh(Literal(Double.PositiveInfinity)), Double.PositiveInfinity, EmptyRow)
1074+
checkEvaluation(Asinh(Literal(Double.NegativeInfinity)), Double.NegativeInfinity, EmptyRow)
1075+
1076+
// acosh: hardcoded reference values cross-verified against C libm (glibc/musl fdlibm)
1077+
checkEvaluation(Acosh(Literal(1.0)), 0.0, EmptyRow)
1078+
checkEvaluation(Acosh(Literal(1.5)), 0.9624236501192069, EmptyRow)
1079+
checkEvaluation(Acosh(Literal(2.0)), 1.3169578969248166, EmptyRow)
1080+
checkEvaluation(Acosh(Literal(10.0)), 2.993222846126381, EmptyRow)
1081+
checkEvaluation(Acosh(Literal(1e8)), 19.11382792451231, EmptyRow)
1082+
// x >= 2^28 branch
1083+
val acoshExpected = Math.log(Double.MaxValue) + StrictMath.log(2.0)
1084+
checkEvaluation(Acosh(Literal(Double.MaxValue)), acoshExpected, EmptyRow)
1085+
checkEvaluation(Acosh(Literal(Double.PositiveInfinity)), Double.PositiveInfinity)
1086+
// x < 1 => NaN
1087+
checkEvaluation(Acosh(Literal(0.5)), Double.NaN, EmptyRow)
1088+
}
1089+
10281090
}

sql/core/src/test/resources/sql-tests/results/postgreSQL/float8.sql.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ SELECT asinh(double('1'))
577577
-- !query schema
578578
struct<ASINH(1):double>
579579
-- !query output
580-
0.8813735870195429
580+
0.881373587019543
581581

582582

583583
-- !query

sql/core/src/test/scala/org/apache/spark/sql/MathFunctionsSuite.scala

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,15 @@ class MathFunctionsSuite extends QueryTest with SharedSparkSession {
145145

146146
test("asinh") {
147147
testOneToOneMathFunction(asinh,
148-
(x: Double) => math.log(x + math.sqrt(x * x + 1)) )
148+
(x: Double) => {
149+
val ax = Math.abs(x)
150+
val w = if (ax.isInfinite || ax.isNaN) ax
151+
else if (ax < 1.0 / (1 << 28)) ax
152+
else if (ax > (1 << 28)) StrictMath.log(ax) + StrictMath.log(2.0)
153+
else if (ax > 2.0) StrictMath.log(2.0 * ax + 1.0 / (math.sqrt(x * x + 1.0) + ax))
154+
else StrictMath.log1p(ax + x * x / (1.0 + math.sqrt(1.0 + x * x)))
155+
Math.copySign(w, x)
156+
})
149157
}
150158

151159
test("cos") {
@@ -167,7 +175,13 @@ class MathFunctionsSuite extends QueryTest with SharedSparkSession {
167175

168176
test("acosh") {
169177
testOneToOneMathFunction(acosh,
170-
(x: Double) => math.log(x + math.sqrt(x * x - 1)) )
178+
(x: Double) => {
179+
if (x < 1.0) Double.NaN
180+
else if (x >= (1 << 28)) StrictMath.log(x) + StrictMath.log(2.0)
181+
else if (x == 1.0) 0.0
182+
else if (x > 2.0) StrictMath.log(2.0 * x - 1.0 / (x + math.sqrt(x * x - 1.0)))
183+
else { val t = x - 1.0; StrictMath.log1p(t + math.sqrt(2.0 * t + t * t)) }
184+
})
171185
}
172186

173187
test("tan") {

0 commit comments

Comments
 (0)