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

Pure-Python mode with pxd: nogil #3170

Open
paugier opened this issue Oct 7, 2019 · 11 comments

Comments

@paugier
Copy link

commented Oct 7, 2019

For the Transonic project, Cython is used via the pure-Python mode with a .pxd for the signatures.

I'd like to support something like (direct application for scikit-image):

# cython: language_level=3
import cython

cdef inline cython.int add(cython.int a, cython.int b) nogil:
    return a + b

cpdef use_add(cython.int n):
    cdef Py_ssize_t i
    cdef cython.int result = 1
    with cython.nogil:
        for i in range(n):
            result = add(result, result)
    return result

I try to translate this in pure-Python with pxd, which gives:

  • the Python file
# cython: language_level=3
import cython

@cython.cfunc
@cython.inline
@cython.nogil
def add(a, b):
    return a + b

@cython.ccall
def use_add(n):
    result = 1
    with cython.nogil:
        for i in range(n):
            result = add(result, result)
    return result
  • the .pxd file
cdef cython.int add(cython.int a, cython.int b)

@cython.locals(n=cython.int, i=Py_ssize_t, result=cython.int)
cpdef use_add(cython.int n)

But Cython complains loudly. I think it's a Cython bug. Can you please tell me if there is a problem in the pure-Python code ? If it is indeed a bug, where should I start to try to fix it ?

The error log:

Compiling pure_with_pxd.py because it changed.
[1/1] Cythonizing pure_with_pxd.py

Error compiling Cython file:
------------------------------------------------------------
...
cdef cython.int add(cython.int a, cython.int b)

@cython.locals(n=cython.int, i=Py_ssize_t, result=cython.int)
^
------------------------------------------------------------

pure_with_pxd.pxd:3:0: Cdef functions/classes cannot take arbitrary decorators.

Error compiling Cython file:
------------------------------------------------------------
...
@cython.ccall
def use_add(n):
    result = 1
    with cython.nogil:
        for i in range(n):
            result = add(result, result)
           ^
------------------------------------------------------------

pure_with_pxd.py:15:12: Assignment of Python object not allowed without gil

Error compiling Cython file:
------------------------------------------------------------
...
import cython

@cython.cfunc
@cython.inline
@cython.nogil
def add(a, b):
^
------------------------------------------------------------

pure_with_pxd.py:7:0: Function with Python return type cannot be declared nogil

Error compiling Cython file:
------------------------------------------------------------
...

@cython.cfunc
@cython.inline
@cython.nogil
def add(a, b):
    return a + b
   ^
------------------------------------------------------------

pure_with_pxd.py:8:4: Returning Python object not allowed without gil

Error compiling Cython file:
------------------------------------------------------------
...

@cython.cfunc
@cython.inline
@cython.nogil
def add(a, b):
    return a + b
            ^
------------------------------------------------------------

pure_with_pxd.py:8:13: Operation not allowed without gil

Error compiling Cython file:
------------------------------------------------------------
...

@cython.ccall
def use_add(n):
    result = 1
    with cython.nogil:
        for i in range(n):
                ^
------------------------------------------------------------

pure_with_pxd.py:14:17: Iterating over Python object not allowed without gil

Error compiling Cython file:
------------------------------------------------------------
...

@cython.ccall
def use_add(n):
    result = 1
    with cython.nogil:
        for i in range(n):
                     ^
------------------------------------------------------------

pure_with_pxd.py:14:22: Calling gil-requiring function not allowed without gil

Error compiling Cython file:
------------------------------------------------------------
...

@cython.ccall
def use_add(n):
    result = 1
    with cython.nogil:
        for i in range(n):
                     ^
------------------------------------------------------------

pure_with_pxd.py:14:22: Constructing Python tuple not allowed without gil
Traceback (most recent call last):
  File "/home/users/augier3pi/.pyenv/versions/3.7.3/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/home/users/augier3pi/.pyenv/versions/3.7.3/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/users/augier3pi/Dev/transonic/transonic_cl/cythonize.py", line 11, in <module>
    ext_modules=cythonize(path, language_level=3), include_dirs=[np.get_include()]
  File "/home/users/augier3pi/.pyenv/versions/3.7.3/lib/python3.7/site-packages/Cython/Build/Dependencies.py", line 1096, in cythonize
    cythonize_one(*args)
  File "/home/users/augier3pi/.pyenv/versions/3.7.3/lib/python3.7/site-packages/Cython/Build/Dependencies.py", line 1219, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: pure_with_pxd.py
@scoder

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

Could you add a cimport cython to the .pxd file and try again?

Also note that in a .pxd file (or any Cython code, really), cython.int is the same as int.

@paugier

This comment has been minimized.

Copy link
Author

commented Oct 8, 2019

Thank you for the advice, but unfortunately I get the same error.

@scoder

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

Looks like a bug then.

It probably works if you remove the .pxd file and use the decorators only in the .py file, right?

@scoder

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

(Also note that the signatures for add() in the .py and .pxd files are conflicting regarding inline and nogil – that should better also be flagged as an error by the compiler, but it seems to be misinterpreting something here which might suppress that error.)

@paugier

This comment has been minimized.

Copy link
Author

commented Oct 9, 2019

Also note that the signatures for add() in the .py and .pxd files are conflicting regarding inline and nogil

If I add inline and nogil to the signature, I get: warning: pure_with_pxd.pxd:3:19: Declarations should not be declared inline.

@paugier

This comment has been minimized.

Copy link
Author

commented Oct 9, 2019

It probably works if you remove the .pxd file and use the decorators only in the .py file, right?

Right. The problem is really with the pxd file. But I real cases, I need the pxd file, in particular to define fused types.

@paugier

This comment has been minimized.

Copy link
Author

commented Oct 9, 2019

I post a new simple version of the example which triggers the same bug:

The Python file:

# cython: language_level=3
import cython

@cython.cfunc
@cython.inline
@cython.nogil
def add(a, b):
    return a + b

@cython.ccall
def use_add(n):
    result = 1
    with cython.nogil:
        for i in range(n):
            result = add(result, result)
    return result

And the .pxd file:

cimport cython

cdef int add(int a, int b) nogil

@cython.locals(n=int, i=int, result=int)
cpdef int use_add(int n)

I get the warning warning: pure_with_pxd.pxd:3:19: Declarations should not be declared inline. and the same errors than in the first message.

Edited: remove the inline in the .pxd file.

@scoder

This comment has been minimized.

Copy link
Contributor

commented Oct 10, 2019

@paugier

This comment has been minimized.

Copy link
Author

commented Oct 10, 2019

I removed the inline in the .pxd file. Unfortunately, even with the nogil I still get:

Error compiling Cython file:
------------------------------------------------------------
...
import cython

@cython.cfunc
@cython.inline
@cython.nogil
def add(a, b):
^
------------------------------------------------------------

pure_with_pxd.py:7:0: Function signature does not match previous declaration

Error compiling Cython file:
------------------------------------------------------------
...
@cython.nogil
def add(a, b):
    return a + b

@cython.ccall
def use_add(n):
^
------------------------------------------------------------

pure_with_pxd.py:11:0: Function signature does not match previous declaration

It would be great to have a little bit more information on why Cython thinks that the "Function signature does not match previous declaration"...

@scoder

This comment has been minimized.

Copy link
Contributor

commented Oct 12, 2019

It would be great to have a little bit more information on why Cython thinks that the "Function signature does not match previous declaration"...

PR welcome.

The "nogil" has to match, though.

Coming back to this, I think it's actually not true either. It's semantically ok to declare the implementation as nogil without making the .pxd declaration nogil as well (because it gives a wider contract to the outside world), just not the other way round (which would allow the outside world to acll the function incorrectly without holding the GIL).

May I ask why you need to duplicate the declaration in the source via decorators that you are already providing in the ,pxd file in Cython syntax? It should work if you remove the decorators and only keep the .pxd override declaration.

(So, the bug is that Cython shouldn't complain, not that there's no way to do this.)

@paugier

This comment has been minimized.

Copy link
Author

commented Oct 12, 2019

It should work if you remove the decorators and only keep the .pxd override declaration.

I confirm that this particular solution works!

But I get the warning message "warning: pure_with_pxd.pxd:3:19: Declarations should not be declared inline." And if i follow the advice, it does not compile.

Anyway, thank you for your help. Do you have a similar solution for this highly related issue #3169 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.