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 3: nogil must be at the end of the function signature line #44

Closed
bdice opened this issue Mar 10, 2023 · 4 comments
Closed

Cython 3: nogil must be at the end of the function signature line #44

bdice opened this issue Mar 10, 2023 · 4 comments

Comments

@bdice
Copy link

bdice commented Mar 10, 2023

I am experimenting with Cython 3.0.0 beta 1. It raises the following warning hundreds of times when cimport-ing cuda-python.

warning: /home/bdice/mambaforge/envs/cudf_cython_beta/lib/python3.10/site-packages/cuda/ccudart.pxd:1123:154: The keyword 'nogil' should appear at the end of the function signature line. Placing it before 'except' or 'noexcept' will be disallowed in a future version of Cython.

I think this expects nogil except ?cudaErrorCallRequiresNewerDriver to be replaced by except ?cudaErrorCallRequiresNewerDriver nogil. If we could fix this, it would be a massive improvement to the compiler diagnostics that are shown during builds, which are otherwise overwhelmed by the sheer volume of this message.

@bdice bdice changed the title Cython 3: nogil must be at the end of the line Cython 3: nogil must be at the end of the function signature line Mar 10, 2023
@bdice
Copy link
Author

bdice commented Mar 16, 2023

@vzhurba01 Do you think this would be possible to fix in the next release of cuda-python? Should be a find-replace. Manually building with Cython 3 would be a good-enough test for me, even if it's not regression tested in CI for now.

@vzhurba01
Copy link
Collaborator

Looks simple enough, lets fix it for the next release. Thanks.

@vzhurba01
Copy link
Collaborator

Resolved in CUDA Python 12.2.0. Closing.

@bdice
Copy link
Author

bdice commented Jun 28, 2023

@vzhurba01 Thank you!

rapids-bot bot pushed a commit to rapidsai/rmm that referenced this issue Aug 4, 2023
This PR contains the minimal set of changes to compile using Cython 3 without warnings. Future PRs can be made to take advantage of new or improved features.

The specific changes are:
- Ensuring `nogil` always comes after `except`. `except * nogil` is a compile-time error in Cython 3
- Adding `noexcept` or `except *` to any `cdef ` functions missing them. In Cython 0.29 these would default to `noexcept`, which meant that exceptions would not be properly propagated. In Cython 3.0.0, these default to `except *`, which incurs a performance penalty for reacquiring the GIL to check the exception value even for `nogil` functions. Being explicit here is important.

There are a large number of outstanding warnings due to NVIDIA/cuda-python#44. cuda-python for CUDA 12 has the necessary fix, but we will need a cuda-python 11.8.* bugfix with a backport to make those warnings go away.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Ashwin Srinath (https://github.com/shwina)
  - Lawrence Mitchell (https://github.com/wence-)
  - Ray Douglass (https://github.com/raydouglass)

URL: #1313
rapids-bot bot pushed a commit to rapidsai/cudf that referenced this issue Aug 4, 2023
This PR contains the minimal set of changes to compile using Cython 3 without warnings. Future PRs can be made to take advantage of new or improved features.

The specific changes are:
- Ensuring `nogil` always comes after `except`. `except * nogil` is a compile-time error in Cython 3
- Removing any extern cdef functions that uses C++ rvalues. These were never supported by Cython, but prior to 3.0 they were silently ignored whereas now Cython throws warnings during compilation
- Relative imports are no longer off by one level in pxd files and must be adjusted accordingly (see cython/cython#3442)

There are a large number of outstanding warnings due to NVIDIA/cuda-python#44. cuda-python for CUDA 12 has the necessary fix, but we will need a cuda-python 11.8.* bugfix with a backport to make those warnings go away.

There are also warnings coming from pyarrow due to apache/arrow#34564. pyarrow 12 contains the necessary fixes, so this issues should be resolved once #13728 is merged.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Ray Douglass (https://github.com/raydouglass)
  - Bradley Dice (https://github.com/bdice)
  - Benjamin Zaitlen (https://github.com/quasiben)
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - https://github.com/jakirkham

URL: #13777
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

No branches or pull requests

2 participants