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.

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

make tests run on numpy1.x and numpy2.x #6076

Merged
merged 4 commits into from Mar 18, 2024
Merged

Conversation

mattip
Copy link
Contributor

@mattip mattip commented Mar 14, 2024

Fixes #6072

Should this also add a numpy2.0b1 test run to CI via the wheels at https://pypi.anaconda.org/scientific-python-nightly-wheels/numpy (i.e. install numpy via this index)?

pip install -i https://pypi.anaconda.org/scientific-python-nightly-wheels/simple numpy

Edit: make it a question

@mattip
Copy link
Contributor Author

mattip commented Mar 14, 2024

This is better but it does not fix everything. There is some mess since on NumPy1

typedef struct { double real, imag; } npy_cdouble;
typedef struct { float real, imag; } npy_cfloat;
typedef struct { npy_longdouble real, imag; } npy_clongdouble;

but on NumPy2

#if defined(__cplusplus)

typedef struct
{
    double _Val[2];
} npy_cdouble;

typedef struct
{
    float _Val[2];
} npy_cfloat;

typedef struct
{
    long double _Val[2];
} npy_clongdouble;

#else
#include <complex.h>
#if defined(_MSC_VER) && !defined(__INTEL_COMPILER)
typedef _Dcomplex npy_cdouble;
typedef _Fcomplex npy_cfloat;
typedef _Lcomplex npy_clongdouble;
#else /* !defined(_MSC_VER) || defined(__INTEL_COMPILER) */
typedef double _Complex npy_cdouble;
typedef float _Complex npy_cfloat;
typedef longdouble_t _Complex npy_clongdouble;
#endif
#endif

This is making compilation fail since cython assumes .real and .imag can be used on complex types.

@mattip
Copy link
Contributor Author

mattip commented Mar 17, 2024

There is no simple way to map np.int_ to a C type on windows, since on NumPy1 it is an alias of np.int32 but on NumPy2 it is an alias to np.int64. On non-windows, it is always an alias to np.int64.

@da-woods
Copy link
Contributor

There is no simple way to map np.int_ to a C type on windows, since on NumPy1 it is an alias of np.int32 but on NumPy2 it is an alias to np.int64. On non-windows, it is always an alias to np.int64.

I think dropping np.int_ is reasonable. Or possibly running the test selectively on Numpy2 only.

Not sure what to do about the complex stuff. There probably isn't a sensible way of writing one test for Cython that covers both (because the interface has changed, and what we're really testing in Cython is the interface)

@mattip
Copy link
Contributor Author

mattip commented Mar 17, 2024

Not sure what to do about the complex stuff.

I think cython should wait for a resolution to numpy/numpy#26029, and adjust the tests appropriately. Clearly arr[1].imag += 1 is wrong, but I am not sure what is right. I would like to have a way to write np_csetimag(np_cimag(arr[1]), np_cimag(arr[1]) + 1) more compactly.

In any case, this PR goes most of the way to fixing the incompatibility in a way that works on both versions, so if CI agrees maybe it could be merged and the rest handled in a subsequent PR.

@mattip
Copy link
Contributor Author

mattip commented Mar 17, 2024

CI is passing.

@da-woods da-woods added this to the 3.1 milestone Mar 18, 2024
@da-woods
Copy link
Contributor

In any case, this PR goes most of the way to fixing the incompatibility in a way that works on both versions, so if CI agrees maybe it could be merged and the rest handled in a subsequent PR.

Yeah I think I'm happy to go along with this.

I'll put off a decision about backporting it to now and just merge it to master I think.

Thanks @mattip

@da-woods da-woods merged commit 4e0eee4 into cython:master Mar 18, 2024
64 checks passed
@mattip mattip mentioned this pull request Mar 20, 2024
da-woods pushed a commit that referenced this pull request Mar 22, 2024
* make tests run on numpy1.x and numpy2.x

* more casting to fix NumPy1/NumPy2 compatibility

* On NumPy2, int_t is now int64_t

* remove test for np.int_
@da-woods
Copy link
Contributor

3.0.x commit e4b1e5e (since it looks like the Python 3.12 tests there use Numpy 2)

@da-woods da-woods modified the milestones: 3.1, 3.0.10 Mar 22, 2024
@mattip
Copy link
Contributor Author

mattip commented Mar 22, 2024

3.0.x commit e4b1e5e

I think #6100 is also needed

@da-woods
Copy link
Contributor

3.0.x commit e4b1e5e

I think #6100 is also needed

Yes I think so too - I still need to have a proper look at that

@@ -22,7 +22,7 @@ def test_parallel_numpy_arrays():
4
"""
cdef Py_ssize_t i, length
cdef np.ndarray[np.int_t] x
cdef np.ndarray[np.int64_t] x

Choose a reason for hiding this comment

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

On numpy 1.26.0 on i686 this test fails with:

Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.12/doctest.py", line 1361, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest numpy_parallel.test_parallel_numpy_arrays[0]>", line 1, in <module>
        test_parallel_numpy_arrays()
      File "tests/run/numpy_parallel.pyx", line 34, in numpy_parallel.test_parallel_numpy_arrays (numpy_parallel.c:4494)
        x = numpy.zeros(10, dtype=numpy.int_)
    ValueError: Buffer dtype mismatch, expected 'int64_t' but got 'long'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Test failures with NumPy 2.0b1
3 participants