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

Cython makes assumptions about the internal structure of arrays preventing use with pypy #3448

Closed
andyDoucette opened this issue Mar 20, 2020 · 5 comments

Comments

@andyDoucette
Copy link

andyDoucette commented Mar 20, 2020

This is a sister post to this entry on the pypy side:

https://bitbucket.org/pypy/pypy/issues/3039/cython-arrayarray-bindings-generate-errors

That was posted by someone else, but I believe I'm getting the same issue. Here's my issue:

I'm trying to use keras in pypy3. Keras depends on h5py. h5py depends on Cython. Cython does not seem to be working in pypy3.

[user@lob cli]$ pypy3
Python 3.6.9 (1608da62bfc7, Dec 23 2019, 10:50:04)
[PyPy 7.3.0 with GCC 7.3.1 20180303 (Red Hat 7.3.1-5)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>>> import h5py
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/root/.local/lib/pypy3.6/site-packages/h5py/__init__.py", line 55, in <module>
    from . import h5a, h5d, h5ds, h5f, h5fd, h5g, h5r, h5s, h5t, h5p, h5z, h5pl
  File ".eggs/Cython-0.29.15-py3.6.egg/Cython/Includes/cpython/array.pxd", line 58, in init h5py.h5d
ValueError: array.array size changed, may indicate binary incompatibility. Expected 72 from C header, got 24 from PyObject
>>>> 

[user@lob cli]$ pypy3 --version
Python 3.6.9 (1608da62bfc7, Dec 23 2019, 10:50:04)
[PyPy 7.3.0 with GCC 7.3.1 20180303 (Red Hat 7.3.1-5)]

For now, I just have a minimal system with minimal requirements to highlight the issue:

[user@lob cli]$ pip list
WARNING: The directory '/root/.cache/pip' or its parent directory is not owned or is not writable by the current user. The cache has been disabled. Check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag.
Package    Version
---------- -------
cffi       1.13.2 
greenlet   0.4.13 
h5py       2.10.0 
numpy      1.18.1 
pip        20.0.2 
readline   6.2.4.1
setuptools 45.1.0 
six        1.14.0 
wheel      0.34.2 

The conclusion in the first thread was:

It seems cython assumes an arrayobject cython/Include/array.pxd will have known ob_size, ob_descr and data, where all we give cython is the PyObject* HEAD (24 bytes). So cython cannnot be sure the other fields exist. It seems cython is willing to “cheat” and assume it knows the inner structure here, which will not work on PyPy.

I'd really like someone's help getting a cython version that will work in pypy. I can probably work with h5py to get it integrated into their package so that keras can run. A lot of data scientist doing machine learning depend on keras, and being able to write fast looped code is a huge benefit that pypy offers. It would be great to have things working in harmony.

Thank you in advance for your consideration. :)

@mattip
Copy link
Contributor

mattip commented Mar 20, 2020

This has been fixed in h5py xref h5py/h5py#1515.

@andyDoucette
Copy link
Author

andyDoucette commented Mar 22, 2020 via email

@scoder
Copy link
Contributor

scoder commented Mar 29, 2020

I think it's ok to consider the array.array internals CPython specific. Using them is not (currently) compatible with PyPy. The decision is then up to the users.

@scoder scoder closed this as completed Mar 29, 2020
@mattip
Copy link
Contributor

mattip commented Mar 29, 2020

Maybe Cython could produce an error message on cimport array.array on PyPy?

@scoder
Copy link
Contributor

scoder commented Mar 30, 2021

Maybe Cython could produce an error message on cimport array.array on PyPy?

I added a PyPy specific C compile time warning in 3c34c40.

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

No branches or pull requests

3 participants