Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BUG: Comparison of longdouble and int causes stack overflow on Windows (pre-release) #22787

Closed
effigies opened this issue Dec 13, 2022 · 4 comments · Fixed by #22789
Closed

BUG: Comparison of longdouble and int causes stack overflow on Windows (pre-release) #22787

effigies opened this issue Dec 13, 2022 · 4 comments · Fixed by #22789
Labels
00 - Bug 06 - Regression
Milestone
1.24.0 release

Comments

@effigies
Copy link
Contributor

effigies commented Dec 13, 2022 •
edited

Describe the issue:

In the 1.24 prerelease builds, I'm seeing a very consistent stack overflow error on Windows CI tests. It occurs when comparing a longdouble to a Python integer.

Note that this is a process crash, not a Python exception. Running the reproduction in a Python shell simply crashes out of the shell, but when running with pytest, I get a notification of Windows fatal exception: stack overflow.

I've just now gotten an environment up where I'm able to test things locally on Windows and build numpy. I've verified that #21875 introduced the failure below, as reverting that behavior allows the test below to run (fail) as expected.

Reproduce the code example:

import numpy as np

def test_1_passes():
    # This actually should fail, but passes under rc2
    assert np.longdouble(2**64) == 2**64 - 1

def test_2_crashes():
    # This should fail as well, but instead crashes
    assert np.longdouble(2**64) == 2**64

if __name__ == "__main__":
    print("Start")
    test_1_passes()
    print("Test 1 passed")
    test_2_crashes()
    print("Test 2 passed")

Error message:

With Python:

$ python min_test.py
Start
Test 1 passed
$

With pytest:

$ pytest -v min_test.py
========================================================== test session starts ==========================================================
platform win32 -- Python 3.10.8, pytest-7.2.0, pluggy-1.0.0 -- C:\Users\marki\Miniconda3\envs\numpy-build\python.exe
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('C:\\Users\\marki\\Projects\\nipy\\nibabel\\.hypothesis\\examples')
rootdir: C:\Users\marki\Projects\nipy\nibabel
plugins: hypothesis-6.61.0, cov-4.0.0, doctestplus-0.12.1, httpserver-1.0.6
collected 2 items

min_test.py::test_1_passes PASSED                                                                                                  [ 50%]
min_test.py::test_2_crashes Windows fatal exception: stack overflow

Current thread 0x00004c48 (most recent call first):
  File "C:\Users\marki\Projects\nipy\nibabel\min_test.py", line 7 in test_2_crashes
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\python.py", line 195 in pytest_pyfunc_call
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_callers.py", line 39 in _multicall
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_manager.py", line 80 in _hookexec
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_hooks.py", line 265 in __call__
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\python.py", line 1789 in runtest
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\runner.py", line 167 in pytest_runtest_call
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_callers.py", line 39 in _multicall
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_manager.py", line 80 in _hookexec
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_hooks.py", line 265 in __call__
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\runner.py", line 260 in <lambda>
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\runner.py", line 339 in from_call
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\runner.py", line 259 in call_runtest_hook
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\runner.py", line 220 in call_and_report
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\runner.py", line 131 in runtestprotocol
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\runner.py", line 112 in pytest_runtest_protocol
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_callers.py", line 39 in _multicall
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_manager.py", line 80 in _hookexec
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_hooks.py", line 265 in __call__
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\main.py", line 349 in pytest_runtestloop
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_callers.py", line 39 in _multicall
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_manager.py", line 80 in _hookexec
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_hooks.py", line 265 in __call__
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\main.py", line 324 in _main
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\main.py", line 270 in wrap_session
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\main.py", line 317 in pytest_cmdline_main
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_callers.py", line 39 in _multicall
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_manager.py", line 80 in _hookexec
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\pluggy\_hooks.py", line 265 in __call__
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\config\__init__.py", line 167 in main
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\site-packages\_pytest\config\__init__.py", line 190 in console_main
  File "C:\Users\marki\Miniconda3\envs\numpy-build\Scripts\pytest.exe\__main__.py", line 7 in <module>
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\runpy.py", line 86 in _run_code
  File "C:\Users\marki\Miniconda3\envs\numpy-build\lib\runpy.py", line 196 in _run_module_as_main

NumPy/Python version information:

1.24.0rc2 3.10.8 | packaged by conda-forge | (main, Nov 24 2022, 14:07:00) [MSC v.1916 64 bit (AMD64)]

Context for the issue:

We have a function as_int() that aims to be more precise than int() on longdoubles, which uses the int(x) == x check to see if it needs to do more work.

xref nipy/nibabel#1147
@seberg
Copy link
Member

seberg commented Dec 13, 2022

Thanks for the nice tracking down! Sorry you went through the bisect I guess, it is a pretty clear reason.

The PR replaced one return value with another by accident and I am surprised I hadn't added a test for the comparison path in the earlier PR :(.

@seberg seberg added this to the 1.24.0 release milestone Dec 13, 2022
@seberg
Copy link
Member

seberg commented Dec 13, 2022

Note that the first result is expected since the numbers are compared using their floating point values by NumPy:

In [3]: np.double(2**64) == 2**64 - 1
Out[3]: True

(You can consider that as incorrect, but it is a different bug from the crash.)

seberg added a commit to seberg/numpy that referenced this issue Dec 13, 2022
@seberg
BUG: Fix infinite recursion in longdouble/large integer scalar ops
5df68d2 
A smal bug snuck in when implementing NEP 50 weak-scalar logic,
and unfortunately the tests didn't cover that specific path :/.

closes numpygh-22787
@seberg seberg mentioned this issue Dec 13, 2022
Merged
@seberg
Copy link
Member

seberg commented Dec 13, 2022

Opened a PR to fix the regression, but you are right of course assert np.longdouble(2**64) == 2**64 should not return False. Even if we say coercion (loss of precision) is fine deferring means we consider the comparison False even if they are exactly the same.

Thus, you are right of course, it is super strange that:

np.float64(2**64) == 2**64-1

loses precision, while:

np.float64(2**64) == 2**64+1

does not, because it takes "the same" path, and ends up using Python's float to int comparison.

(I am not sure we should worry about the imprecise behavior, but the inconsistency is arguably weird.)

@effigies
Copy link
Contributor Author

Yeah, I agree that it's weird. But at least it's consistent with the behavior in numpy 1.23. Running the above example code fails in both tests again:

Expand pytest outputs 
> pytest -v .\min_test.py
======================================= test session starts ========================================
platform win32 -- Python 3.10.8, pytest-7.2.0, pluggy-1.0.0 -- C:\Users\marki\Miniconda3\envs\numpy-build\python.exe
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('C:\\Users\\marki\\Projects\\nipy\\nibabel\\.hypothesis\\examples')
rootdir: C:\Users\marki\Projects\nipy\nibabel
plugins: hypothesis-6.61.0, cov-4.0.0, doctestplus-0.12.1, httpserver-1.0.6
collected 2 items

min_test.py::test_1_passes FAILED                                                             [ 50%]
min_test.py::test_2_crashes FAILED                                                            [100%]

============================================= FAILURES =============================================
__________________________________________ test_1_passes ___________________________________________

    def test_1_passes():
        # This actually should fail, but passes under rc2
>       assert np.longdouble(2**64) == 2**64 - 1
E       AssertionError: assert 1.8446744073709552e+19 == ((2 ** 64) - 1)
E        +  where 1.8446744073709552e+19 = <class 'numpy.longdouble'>((2 ** 64))
E        +    where <class 'numpy.longdouble'> = np.longdouble

min_test.py:5: AssertionError
__________________________________________ test_2_crashes __________________________________________

    def test_2_crashes():
        # This should fail as well, but instead crashes
>       assert np.longdouble(2**64) == 2**64
E       AssertionError: assert 1.8446744073709552e+19 == (2 ** 64)
E        +  where 1.8446744073709552e+19 = <class 'numpy.longdouble'>((2 ** 64))
E        +    where <class 'numpy.longdouble'> = np.longdouble

min_test.py:9: AssertionError
===================================== short test summary info ======================================
FAILED min_test.py::test_1_passes - AssertionError: assert 1.8446744073709552e+19 == ((2 ** 64) - 1)
FAILED min_test.py::test_2_crashes - AssertionError: assert 1.8446744073709552e+19 == (2 ** 64)
======================================== 2 failed in 0.30s =========================================

#22789 does fix our tests that cover the cases we are concerned about, so from my perspective this is correct enough.

charris pushed a commit to charris/numpy that referenced this issue Dec 13, 2022
@seberg @charris
BUG: Fix infinite recursion in longdouble/large integer scalar ops
21f7096 
A smal bug snuck in when implementing NEP 50 weak-scalar logic,
and unfortunately the tests didn't cover that specific path :/.

closes numpygh-22787
@charris charris mentioned this issue Dec 13, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
00 - Bug 06 - Regression
Projects
None yet
Milestone
1.24.0 release
Development

Successfully merging a pull request may close this issue.

BUG: Fix infinite recursion in longdouble/large integer scalar ops seberg/numpy
2 participants
@seberg @effigies