gemini-code-assist[bot] commented on code in PR #38871:
URL: https://github.com/apache/beam/pull/38871#discussion_r3404732992
##########
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/transform/agg/VarianceFn.java:
##########
@@ -133,7 +147,15 @@ public Coder<VarianceAccumulator> getAccumulatorCoder(
@Override
public T extractOutput(VarianceAccumulator accumulator) {
- return decimalConverter.apply(getVariance(accumulator));
+ BigDecimal result = getVariance(accumulator);
+ if (result != null && isStddev) {
+ // Take the square root in IEEE double precision so the result matches
Spark / numpy bit for
+ // bit (both compute stddev as Math.sqrt over a double).
BigDecimal.sqrt(MATH_CTX) would round
+ // to 10 significant digits and fail the test harness's exact comparison
of standard
+ // deviation.
+ result = BigDecimal.valueOf(Math.sqrt(result.doubleValue()));
+ }
Review Comment:
If `getVariance(accumulator)` returns a negative value (which can happen due
to numerical instability or rounding errors when computing variance, e.g., when
all values are nearly identical), `result.doubleValue()` will be negative.
Taking `Math.sqrt()` of a negative number yields `Double.NaN`.
When `BigDecimal.valueOf(Double.NaN)` is called, it internally calls
`Double.toString(Double.NaN)` and attempts to parse `"NaN"`, which throws a
`NumberFormatException`.
Similarly, if `result.doubleValue()` overflows to
`Double.POSITIVE_INFINITY`, `BigDecimal.valueOf` will throw a
`NumberFormatException` for `"Infinity"`.
To prevent these runtime crashes, we should clamp negative values to `0.0`
and handle non-finite double values safely.
```suggestion
BigDecimal result = getVariance(accumulator);
if (result != null && isStddev) {
// Take the square root in IEEE double precision so the result matches
Spark / numpy bit for
// bit (both compute stddev as Math.sqrt over a double).
BigDecimal.sqrt(MATH_CTX) would round
// to 10 significant digits and fail the test harness's exact
comparison of standard
// deviation.
double doubleVal = result.doubleValue();
if (doubleVal < 0.0) {
doubleVal = 0.0;
}
double sqrt = Math.sqrt(doubleVal);
if (!Double.isFinite(sqrt)) {
throw new ArithmeticException("Standard deviation result is
non-finite: " + sqrt);
}
result = BigDecimal.valueOf(sqrt);
}
```
--
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]
