[SPARK-58553][PS][FOLLOWUP] Do not cast NumPy fmax/fmin results to double - #58366
Closed
Spenserrrr wants to merge 1 commit into
Closed
[SPARK-58553][PS][FOLLOWUP] Do not cast NumPy fmax/fmin results to double#58366Spenserrrr wants to merge 1 commit into
Spenserrrr wants to merge 1 commit into
Conversation
…uble Every branch of _fmax_func and _fmin_func returns one of the operands, so the trailing cast only widened an already exact answer: an integral column above 2^53 lost precision and the output dtype did not match NumPy's.
Spenserrrr
marked this pull request as ready for review
August 28, 2026 00:04
Contributor
Author
|
Hi @zhengruifeng, this is a follow-up of the integer-precision fixes, similar to fmod (#58319) and floor_divide (#58306). Could you take a look when you have time? Thanks! |
zhengruifeng
approved these changes
Aug 28, 2026
zhengruifeng
pushed a commit
that referenced
this pull request
Aug 28, 2026
…uble ### What changes were proposed in this pull request? `_fmax_func` and `_fmin_func` no longer cast their result to double. Every branch of both expressions returns one of the operands, so the result now keeps the operands' type instead of being widened. ### Why are the changes needed? The cast rounds integral values above 2^53 and returns a dtype NumPy does not use: ```python np.fmax(psdf.x1, psdf.x2) # 9007199254740992.0, float64 np.fmax(pdf.x1, pdf.x2) # 9007199254740993, int64 ``` ### Does this PR introduce _any_ user-facing change? Yes. `np.fmax` / `np.fmin` on a non-double column now return the operands' type rather than double, and integral values above 2^53 are exact. Floating-point results, including the signed-zero tie, are unchanged. ### How was this patch tested? Two new tests in `test_numpy_compat.py`, inherited by the Spark Connect parity suite; both fail without the change. Neither is gated by `_skip_if_numpy_differs`, since nothing they compare varies by platform or NumPy version (checked on the minimum dependencies and on pandas 3). ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) Closes #58366 from Spenserrrr/numpy-trunc-fmax-fmin-precision. Authored-by: Spenser Sun <hsun112358@gmail.com> Signed-off-by: Ruifeng Zheng <ruifengz@foxmail.com> (cherry picked from commit 77a4781) Signed-off-by: Ruifeng Zheng <ruifengz@foxmail.com>
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
_fmax_funcand_fmin_funcno longer cast their result to double. Every branch of both expressions returns one of the operands, so the result now keeps the operands' type instead of being widened.Why are the changes needed?
The cast rounds integral values above 2^53 and returns a dtype NumPy does not use:
Does this PR introduce any user-facing change?
Yes.
np.fmax/np.fminon a non-double column now return the operands' type rather than double, and integral values above 2^53 are exact. Floating-point results, including the signed-zero tie, are unchanged.How was this patch tested?
Two new tests in
test_numpy_compat.py, inherited by the Spark Connect parity suite; both fail without the change. Neither is gated by_skip_if_numpy_differs, since nothing they compare varies by platform or NumPy version (checked on the minimum dependencies and on pandas 3).Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)