-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
[R] Use array interface for dense DMatrix creation #9816
Conversation
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.
Thank you for working on the dense matrix, let me make some changes tomorrow for using the JSON class in XGBoost.
Hi @david-cortes , I have made some changes to use the |
Thanks for the changes. I've made some very small modifications on top and I guess it should be good to go now. |
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.
Nice work! Will merge once the CI is green.
ref #9810
This PR switches the underlying C function for DMatrix creation from dense R matrices to the
XGDMatrixCreateFromDense
function used in the python interface.Along the way, it also fixes a potential memory leak in case of failures, in which a failure to allocate an R externalptr would lead to a longjump without freeing the allocated C++ object from its pointer.
I see there was already some mechanism to create the kind of array JSON that numpy uses in the DMatrix creators from sparse matrices, but couldn't find any equivalent for dense matrices. There also seems to be a dedicated module to create JSONs, but they look simple enough for
snprintf
.The JSON parser for the config by the way seems to have issues with
nan
andinf
, so this PR makes a conversion toNaN
andInfinity
which seem to be accepted.