search

NVIDIA / cuda-python

CUDA Python Low-level Bindings
https://nvidia.github.io/cuda-python/
Other
859 stars 68 forks source link

Use python exceptions instead of `err, ... =` #9

Closed h-vetinari closed 2 years ago

h-vetinari commented 2 years ago

Congratulations on the GA release! 🥳

I've been looking forward to the cuda bindings for a while, and was just looking through the docs.

The overview notes an implementation of ASSERT_DRV, which already contains the caveat:

In a future release, this may automatically raise exceptions using a Python object model.

I'm not sure if that means that the errors are going to be subclasses of something like a CUDAError, or if that is to be interpreted some other way, but in any case, I was quite surprised about this choice of exception API

Why not make the functions raise err by default? Right now, IIUC, every invocation would need to accept an extra err-return (and handle it with something like ASSERT_DRV). This seems like a really onerous task to achieve the default behaviour of "fail in case of something unexpected" (and actively choosing where to introduce try... except: handling to continue even if things fail).

It seems like a bad trade-off for me (high verbosity, and easy to forget adding an ASSERT_DRV), but maybe I'm overlooking something?

The reasons I'm raising this right now, is that this would be a pretty fundamental API change, and if there's any chance at all (assuming it's not already "zero" after GA), it would be ASAP.

shwina commented 2 years ago

There will be a higher-level "object oriented" API on top of the current CUDA Python bindings, which will raise exceptions on errors. For efficiency reasons, and to match the C API as closely as possible, the lower level bindings return error codes.

h-vetinari commented 2 years ago

Thanks for the quick response @shwina, proximity to the C-API is something understandable, and I'm very happy to hear that "this may automatically raise exceptions using a Python object model" should be interpreted as an entire higher-level API layer. 😊

(though perhaps that plan could be delineated more clearly in the overview 🙃)

vzhurba01 commented 2 years ago

(though perhaps that plan could be delineated more clearly in the overview :upside_down_face:)

Updated overview as per suggestion: https://nvidia.github.io/cuda-python/overview.html#future-of-cuda-python Closing.