Add python bindings for kmeans fit #1016

benfred · 2022-11-15T01:48:02Z

As a first attempt at exposing the kmeans inertia - this adds python bindings for the kmeans::fit function, which returns the inertia as part of the results.

This also adds support for accessing mdspan api's from cython directly

As a first attempt at exposing the kmeans inertia - this adds python bindings for the kmeans::fit function, which returns the inertia as part of the results. This also adds support for accessing mdspan api's from cython directly

cjnolet

This looks really clean and, other than a few quirks that I think Cython introduces with the typing, I think this really looks like more of a blend of Python + C++ than having to use raw pointers for everything. Overall, I like it.

cjnolet · 2022-11-16T17:38:43Z

python/pylibraft/pylibraft/cluster/kmeans.pyx

+    cdef handle_t *h = <handle_t*><size_t>handle.getHandle()
+
+    x_cai = X.__cuda_array_interface__
+    dtype = np.dtype(x_cai["typestr"])


We should probably have some input validation here- X.shape[1] == centroids.shape[1], n_clusters > 0, etc...

I'm validating the shape/dtype/c-contiguous values - and checking for things like the n_clusters > 0 , and # of features being consistent between X and centroids should be done in the C++ code already .

python/pylibraft/pylibraft/cluster/kmeans.pyx

cjnolet · 2022-11-16T21:21:13Z

cpp/include/raft_distance/kmeans.hpp

+void fit(handle_t const& handle,
+         const KMeansParams& params,


This is really nice. And I like that this gets us closer to ditching the pointer-based APIs.

cpp/src/distance/kmeans_fit_double.cu

cpp/src/distance/kmeans_fit_float.cu

python/pylibraft/pylibraft/common/mdspan.pyx

cjnolet · 2022-11-16T21:33:22Z

python/pylibraft/pylibraft/test/test_kmeans.py

+
+    centroids, inertia, n_iter = fit(params, device_ndarray(X))
+
+    # TODO: validate that centroids are reasonable ... somehow


There's a few different ways to verify this. One way could be to wrap the make_blobs primitive and generate n_clusters blobs, that way we have label information that we can use to compare against each point's nearest centroid.

cjnolet · 2022-11-16T21:34:25Z

python/pylibraft/pylibraft/common/mdspan.pyx

+
+
+cdef device_vector_view[ElementType, int] \
+        device_vector_view_from_array(arr, ElementType * p) except +:


Is it possible for the function to accept type information directly? Like device_vector_view_from_array[ElementType](arr)?

cjnolet · 2022-11-16T21:36:22Z

python/pylibraft/pylibraft/common/mdspan.pyx

+    return make_device_matrix_view(<ElementType*>ptr, <int>rows, <int>cols)
+
+
+cdef device_matrix_view[const ElementType, int] \


I do really like this because it makes it really easy to pass in an array and have it extract the underlying pointer. One thing that stinks here, though, is that the device_matrix_view needs to have type information associated to it at compile-time while the arr's type is only known at runtime.

cjnolet · 2022-11-16T21:38:45Z

python/pylibraft/pylibraft/cluster/kmeans.pyx

+            const_device_matrix_view_from_array[float](X, <float*>NULL),
+            f_sample_weights,
+            device_matrix_view_from_array[float](centroids, <float*>NULL),
+            make_host_scalar_view[float, int](&f_inertia),


This part looks really clean, honestly.

cjnolet · 2022-11-17T16:59:45Z

python/pylibraft/pylibraft/cluster/kmeans.pyx


 from pylibraft.common.input_validation import *
 from pylibraft.distance import DISTANCE_TYPES

+ctypedef const float const_float
+

 def is_c_cont(cai, dt):


This should be in the input_validation utils in pylibraft.common now

Rather than wrap in a function using FusedTypes, use make_device_matrix_view directly

cjnolet · 2022-11-21T16:17:22Z

rerun tests

cjnolet · 2022-11-21T20:00:52Z

rerun tests

benfred · 2022-11-28T23:48:38Z

I changed the branch back to 22.12 until we get the 23.03 branch caught up - which I took a stab at here #1051

cjnolet

I think this is almost there. Just two minor things and I think it's ready to merge. I'm excited to see the pointer-based runtime APIs disappear!

cjnolet · 2022-11-30T01:01:15Z

python/pylibraft/pylibraft/test/test_kmeans.py

+    # TODO: use a fixed RNG state on params
+    params = KMeansParams(n_clusters=n_clusters)
+
+    # fit the centroids, make sure inertia has gone down


Can you create a github issue to expose make_blobs in a future PR and reference it here in this comment?

Ultimately, the make_blobs allows us to generate Gaussian clusters that we can directly compare the output against. I think it's out of scope for this PR but we definitely want to revisit.

Created #1059 - and added a TODO in the test

cjnolet · 2022-11-30T01:02:20Z

python/pylibraft/pylibraft/common/cai_wrapper.py

@@ -71,3 +71,19 @@ def data(self):
        Returns the data pointer of the underlying CUDA array interface
        """
        return self.cai_["data"][0]
+
+    def validate(self, expected_dims=None, expected_dtype=None):


I like this function but I think the name could be a little more descriptive. Maybe something like validate_shape_dtype?

done in last commit

* Add seed/metric/verbosity to python kmeans params * Add a todo about make_blobs testing * rename cai_wrapper.validate to validate_shape_dtype

cjnolet

LGTM!

cjnolet · 2022-11-30T20:08:14Z

@gpucibot merge

Add python bindings for kmeans fit

b0c33a9

As a first attempt at exposing the kmeans inertia - this adds python bindings for the kmeans::fit function, which returns the inertia as part of the results. This also adds support for accessing mdspan api's from cython directly

benfred requested review from a team as code owners November 15, 2022 01:48

benfred marked this pull request as draft November 15, 2022 01:48

github-actions bot added CMake cpp python labels Nov 15, 2022

benfred added 3 commits November 15, 2022 13:13

style

efb0b30

fix

31d232c

pass sample_weights

6d26221

cjnolet requested changes Nov 16, 2022

View reviewed changes

cjnolet reviewed Nov 17, 2022

View reviewed changes

benfred added 3 commits November 18, 2022 10:43

Merge remote-tracking branch 'origin/branch-22.12' into cython_kmeans

52320c4

Use make_device_matrix_view directly

3183345

Rather than wrap in a function using FusedTypes, use make_device_matrix_view directly

style

0aad8d0

cjnolet added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Nov 21, 2022

benfred changed the base branch from branch-22.12 to branch-23.02 November 21, 2022 20:20

benfred added 4 commits November 28, 2022 15:20

Merge branch 'branch-22.12' into cython_kmeans

4e23eaa

use distance specializations to speed up compile time

732e43f

update docstring

9984868

Test inertia / fix issues

5994b52

github-actions bot added the gpuCI label Nov 28, 2022

benfred changed the base branch from branch-23.02 to branch-22.12 November 28, 2022 23:47

build fix

89a5c15

github-actions bot removed the gpuCI label Nov 29, 2022

cjnolet changed the base branch from branch-22.12 to branch-23.02 November 29, 2022 21:50

cjnolet requested changes Nov 30, 2022

View reviewed changes

cjnolet mentioned this pull request Nov 30, 2022

ANN refinement Python wrapper #1052

Merged

benfred mentioned this pull request Nov 30, 2022

[FEA] Expose make_blobs to python #1059

Open

Add parameters / code review feedback

fb3db0f

* Add seed/metric/verbosity to python kmeans params * Add a todo about make_blobs testing * rename cai_wrapper.validate to validate_shape_dtype

benfred marked this pull request as ready for review November 30, 2022 19:33

cjnolet approved these changes Nov 30, 2022

View reviewed changes

rapids-bot bot merged commit 070cca2 into rapidsai:branch-23.02 Nov 30, 2022

benfred deleted the cython_kmeans branch November 30, 2022 21:12

benfred self-assigned this Dec 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add python bindings for kmeans fit #1016

Add python bindings for kmeans fit #1016

benfred commented Nov 15, 2022

cjnolet left a comment

cjnolet Nov 16, 2022

benfred Nov 30, 2022

cjnolet Nov 16, 2022

cjnolet Nov 16, 2022

cjnolet Nov 16, 2022

cjnolet Nov 16, 2022

cjnolet Nov 16, 2022

cjnolet Nov 17, 2022

cjnolet commented Nov 21, 2022

cjnolet commented Nov 21, 2022

benfred commented Nov 28, 2022 •

edited

Loading

cjnolet left a comment

cjnolet Nov 30, 2022

benfred Nov 30, 2022

cjnolet Nov 30, 2022

benfred Nov 30, 2022

cjnolet left a comment

cjnolet commented Nov 30, 2022


		centroids, inertia, n_iter = fit(params, device_ndarray(X))

		# TODO: validate that centroids are reasonable ... somehow



		cdef device_vector_view[ElementType, int] \
		device_vector_view_from_array(arr, ElementType * p) except +:

		return make_device_matrix_view(<ElementType*>ptr, <int>rows, <int>cols)


		cdef device_matrix_view[const ElementType, int] \

Add python bindings for kmeans fit #1016

Add python bindings for kmeans fit #1016

Conversation

benfred commented Nov 15, 2022

cjnolet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjnolet commented Nov 21, 2022

cjnolet commented Nov 21, 2022

benfred commented Nov 28, 2022 • edited Loading

cjnolet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjnolet left a comment

Choose a reason for hiding this comment

cjnolet commented Nov 30, 2022

benfred commented Nov 28, 2022 •

edited

Loading