-
Notifications
You must be signed in to change notification settings - Fork 550
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
[REVIEW] Handle C++ exception thrown from FIL predict #3061
Conversation
Please update the changelog in order to start CI tests. View the gpuCI docs here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! what will happen once #2894 is merged and it stops throwing the exception? Will you need to fully revert it? (does not sound like a problem per se, just wondering)
@levsnv No, we just need to remove the unit test |
Personally, I am paranoid about letting a C++ exception escape the language boundary, because it leads to segfault. In XGBoost, we catch every single C++ exception and convert it to Python exceptions. |
Codecov Report
@@ Coverage Diff @@
## branch-0.17 #3061 +/- ##
===============================================
+ Coverage 59.22% 59.23% +0.01%
===============================================
Files 142 142
Lines 8966 8966
===============================================
+ Hits 5310 5311 +1
+ Misses 3656 3655 -1
Continue to review full report at Codecov.
|
Currently, attempting to use
predict_proba()
with a multi-class classifier will result into a segfault, because a C++ exception is not properly handled in the Python layer.Use
except +
construct in Cython to convert C++ exceptions from thepredict()
function of FIL into Python exceptions.