dongjoon-hyun commented on code in PR #57898:
URL: https://github.com/apache/spark/pull/57898#discussion_r3750692482
##########
python/pyspark/pandas/numpy_compat.py:
##########
@@ -118,6 +118,26 @@ def _fmod_func(c1: Column, c2: Column) -> Column:
)
+def _logaddexp_func(c1: Column, c2: Column, base2: bool = False) -> Column:
+ c1_double = c1.cast("double")
+ c2_double = c2.cast("double")
+ difference = F.abs(c1_double - c2_double)
+ maximum = F.greatest(c1_double, c2_double)
+ if base2:
+ log_term = F.log1p(F.pow(F.lit(2.0), -difference)) / F.log(F.lit(2.0))
+ else:
+ log_term = F.log1p(F.exp(-difference))
+
+ return (
+ F.when(c1_double.isNull() | F.isnan(c1_double), c1_double)
+ .when(c2_double.isNull() | F.isnan(c2_double), c2_double)
+ .when((c1_double == float("inf")) | (c2_double == float("inf")),
F.lit(float("inf")))
+ .when(c1_double == float("-inf"), c2_double)
+ .when(c2_double == float("-inf"), c1_double)
Review Comment:
`np.logaddexp(-np.inf, -0.0)` returns `+0.0` in NumPy (it computes `y +
log1p(exp(x - y)) = -0.0 + 0.0 = +0.0`), but these branches pass the other
operand through unchanged, so this implementation returns `-0.0` instead. The
previous pandas UDF implementation returned `+0.0` here since it called NumPy
directly. Adding `+ F.lit(0.0)` normalizes `-0.0` to `+0.0` like NumPy while
leaving every other value unchanged (NaN/null are already handled by the
earlier branches):
```suggestion
.when(c1_double == float("-inf"), c2_double + F.lit(0.0))
.when(c2_double == float("-inf"), c1_double + F.lit(0.0))
```
##########
python/pyspark/pandas/tests/test_numpy_compat.py:
##########
@@ -246,6 +246,49 @@ def test_np_fmod(self):
self.assert_eq(np.fmod(psdf.x1, psdf.x2), np.fmod(pdf.x1, pdf.x2),
almost=True)
+ def test_np_logaddexp(self):
+ for pdf in (
+ pd.DataFrame(
+ {
+ "x1": [-64, -2, -1, 0, 1, 2, 64],
+ "x2": [2, 3, -2, -3, -3, 0, 2],
+ }
+ ),
+ pd.DataFrame(
+ {
+ "x1": [
+ -np.inf,
+ -np.inf,
+ -2.0,
+ -2.0,
+ -0.0,
+ 0.0,
+ 2.0,
+ np.inf,
+ np.inf,
+ np.nan,
+ -1000.0,
+ ],
+ "x2": [
+ -np.inf,
+ 3.0,
+ -np.inf,
+ 2.0,
+ 0.0,
+ -0.0,
+ np.inf,
+ 2.0,
+ np.inf,
+ 2.0,
+ 1000.0,
+ ],
+ }
+ ),
+ ):
+ psdf = ps.from_pandas(pdf)
+ for np_func in (np.logaddexp, np.logaddexp2):
+ self.assert_eq(np_func(psdf.x1, psdf.x2), np_func(pdf.x1,
pdf.x2), almost=True)
Review Comment:
The test data has `(-0.0, 0.0)` / `(0.0, -0.0)` pairs, but both take the
finite path (result `log(2)`), so the signed-zero behavior of the `-inf`
branches is untested — there is no `(-np.inf, -0.0)` pair, where this
implementation returns `-0.0` while NumPy returns `+0.0`. Also, `assert_eq(...,
almost=True)` does not distinguish `-0.0` from `+0.0`. Could you add a
`(-np.inf, -0.0)` row and assert the sign explicitly (e.g. via `np.signbit` on
the results), similar to the SPARK-58553 follow-up for `fmax`/`fmin`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
