Skip to content

BUG: Fix np.abs(nan) returning negative NaN on wasm32/Pyodide - #31433

Merged
seberg merged 5 commits into
numpy:mainfrom
AnkitAhlawat7742:Fix/numpy_abs_with_numpy_nan
May 20, 2026
Merged

BUG: Fix np.abs(nan) returning negative NaN on wasm32/Pyodide#31433
seberg merged 5 commits into
numpy:mainfrom
AnkitAhlawat7742:Fix/numpy_abs_with_numpy_nan

Conversation

@AnkitAhlawat7742

@AnkitAhlawat7742 AnkitAhlawat7742 commented May 14, 2026

Copy link
Copy Markdown
Contributor

Fix: #31421

PR summary

This PR fixes a bug where np.signbit(np.abs(np.nan)) incorrectly returns True instead of False.

problem

The LONGDOUBLE_absolute function in numpy/_core/src/umath/loops.c.src used tmp + 0 to clear -0.0, but this operation corrupted the sign bit of NaN values, causing np.signbit(np.abs(np.nan)) to return True.

Testing

Manually run the new test case and run all the numpy/_core/tests/test_umath.py file test cases.

@charris

charris commented May 14, 2026

Copy link
Copy Markdown
Member

I want to know why adding 0.0 corrupted the nan sign, any ideas? Was it possible that the nan was already negative?

@AnkitAhlawat7742

AnkitAhlawat7742 commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

HI @charris ,

The bug occurs because nan > 0 always returns falseso the code takes the else branch: tmp = -in1 as per the existing code
For positive NaN:

  • tmp = -(+nan) = -nan (flips the sign bit)
  • tmp + 0 = -nan (the +0 doesn't fix NaN sign bits)
  • Result: negative NaN

So + 0 was meant to clear -0.0, but it doesn't correct NaN sign bits so i checked npy_fabsl() fixes this issue because it's designed to handle NaN
Hope i debug the issue in right direction

@juntyr

juntyr commented May 15, 2026

Copy link
Copy Markdown
Contributor

#31421 (comment)

Hi @juntyr , Did you also observe this issue with float32 and float64 types on wasm32/Pyodide? I noticed the same tmp + 0 pattern exists in those implementations as well. Need to check with reviewer that , can we create separate PR for other types (float32,float64`)...if reviewer agree with this solution

I noticed the problem on WASM with float32, float64, and float128; but surprisingly float16 works fine. Could the PR be amended to fix those as well?

@seberg

seberg commented May 15, 2026

Copy link
Copy Markdown
Member

float16 uses a custom bit-wise implementation for all of these. But yea, I guess this code predates the abs being mandatory functions. If the float abs also uses comparison, I think we might as well change it.
(Not sure what to think if wasm abs might just doesn't flip the signbit for NaN...)

@AnkitAhlawat7742

AnkitAhlawat7742 commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the feedback! I have amended the PR to fix float32 and float64 as well.they had the samea > 0 ? a : -apattern in loops_unary_fp.dispatch.c.src and modified the test case accordingly
@seberg please review it once again and let me know if any modification needed

@AnkitAhlawat7742

Copy link
Copy Markdown
Contributor Author

HI @seberg ,
Please review the PR ..Happy to make any changes if needed!

Comment thread numpy/_core/tests/test_umath.py Outdated
Comment thread numpy/_core/tests/test_umath.py Outdated
Comment thread numpy/_core/tests/test_umath.py Outdated
assert_(np.signbit(np.nan) == 0)
def test_abs_nan_signbit():
"""#31421 abs(nan) preserves positive sign bit correctly."""
for dtype in [np.float16, np.float32, np.float64, np.longdouble]:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could parametrize for better feedback, although array test below would then make sense to split off

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback ..Test case has been updated accordingly ,I have parametrized the test for better feedback, split the array test into a separate function.

Comment thread numpy/_core/tests/test_umath.py Outdated

@seberg seberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM, let's put it in, since C99 guarantees abs being defined.

I am not sure that C99 actually guarantees the sign of NaN (or that we should), but in practice I don't see anything plausibly diverging.

@seberg
seberg merged commit dc478c5 into numpy:main May 20, 2026
86 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: np.abs(np.nan) on Pyodide WASM returns -np.nan

5 participants