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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
h-vetinari opened this issue Oct 21, 2021 · 3 comments
Closed

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

h-vetinari opened this issue Oct 21, 2021 · 3 comments

Comments

@h-vetinari
Copy link

h-vetinari commented Oct 21, 2021

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
Copy link

shwina commented Oct 21, 2021

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
Copy link
Author

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
Copy link
Collaborator

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

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

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

3 participants