[HELP-REQ] Expose KMeans init_plus_plus in pylibraft
#1198
Conversation
| raft::device_matrix_view<double, int> centroids) | ||
| { | ||
| // XXX what should the size of this be? | ||
| rmm::device_uvector<char> workspace(10, handle.get_stream()); |
There was a problem hiding this comment.
What is a good size for this temporary workspace? Does it even matter or is it anyway resized to the size that is needed inside the function(s) that use it?
There was a problem hiding this comment.
In general we have been working to remove the temporary workspace arguments from our public APIs in favor of using the "workspace_resource" in the device_resources instance. However, we can keep this one the way it is in the meantime until we scrape through and clean it up. The device_uvector is an RAII compliant (owns the memory, can resize it, and frees it when destructed) so you can just pass in a 0 size workspace and it'll get resized accordingly inside the init_plus_plus function.
betatim
force-pushed
the
add-init-plus-plus-pylibraft
branch
from
95ea557 to
2a8de87
Compare
cjnolet
added
improvement
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## branch-23.04 #1198 +/- ##
===============================================
Coverage ? 87.99%
===============================================
Files ? 21
Lines ? 483
Branches ? 0
===============================================
Hits ? 425
Misses ? 58
Partials ? 0 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
/merge |
Closes #1166
This adds
init_plus_plusto pylibraft.Three things to discuss:
workspacebe? I couldn't quite work out if the size mattered or not.@property.settertoKMeansParamsso that I can set theseedafter constructing it. But that leads to an error from cython(?). Does anyone know more about how to do this?