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

Propagate exceptions raised in Python callback functions #1096

Merged
merged 4 commits into from
Sep 27, 2022

Conversation

madsbk
Copy link
Member

@madsbk madsbk commented Sep 22, 2022

Description

Currently, we ignore Python exceptions raised in callback functions called by resources such as CallbackMemoryResource. This is because Cython doesn't automatically translate Python exceptions into C++ exceptions (even though Cython does translate C++ exceptions into Python automatically).

To address this issue, we now translate Python exceptions raised in the callback function into a C++ exception manually. Next, we remove Cython's automatic C++ to Python exception translation by not using the except + declaration. Thus, when the callback raises an exception, the caller (a memory resource implemented in C++) can catch and/or propagate the exception.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@madsbk madsbk requested a review from a team as a code owner September 22, 2022 18:02
@github-actions github-actions bot added the Python Related to RMM Python API label Sep 22, 2022
madsbk and others added 2 commits September 23, 2022 10:39
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
@shwina shwina added non-breaking Non-breaking change bug Something isn't working labels Sep 23, 2022
@madsbk
Copy link
Member Author

madsbk commented Sep 26, 2022

@bdice anything else needed here?

Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggestion for improvement here, then I can approve. Bradley is out for most of this week.

python/rmm/_lib/memory_resource.pyx Outdated Show resolved Hide resolved
Co-authored-by: Vyas Ramasubramani <vyas.ramasubramani@gmail.com>
@vyasr
Copy link
Contributor

vyasr commented Sep 27, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 42fd22b into rapidsai:branch-22.10 Sep 27, 2022
@madsbk madsbk deleted the callback_mr_handle_exception branch May 17, 2024 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change Python Related to RMM Python API
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants