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

Throw errors on allocation failures #5166

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

joeyballentine
Copy link
Contributor

@joeyballentine joeyballentine commented Nov 22, 2023

I figure this one might be controversial, and I'm open to suggestions on how to change this.

When I first made my ncnn_vulkan fork, I noticed that when memory allocation or inference failures would happen, they would just silently fail and log to the console. This made any kind of error handling on the python side impossible, as there was no error being thrown to catch and check for. There was also a specific one of these that certain users only ran into sometimes, and I needed a way to specifically handle that case.

So, my solution was to throw errors in the places that I needed to catch those errors. I have ported those here.

However, I don't know if there was maybe a different way I could have accomplished that same goal, and throwing errors in these places I'm sure could cause unexpected behavior in dependent projects. So, I figured I'd open this one as a draft in hopes that we can reach an agreement on what to do here.

For an example of how I'm using this, look here

(and yes, these get automatically bubbled-up to the python bindings)

@github-actions github-actions bot added the core label Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant