Add public API and tests for hierarchical balanced k-means #1113
Conversation
… improve doxygens
Nyrio
added
3 - Ready for Review
feature request
Nyrio
added
the
4 - Waiting on Reviewer
That's covered in the new unit test. |
…commit-hook clang-format...
…rite_only_unary_op` (#1149) Follows [a discussion](#1113 (comment)) that we had about `write_only_unary_op` / `writeOnlyUnaryOp`. For consistency and to simplify the use of this primitive, we shouldn't require dereferencing a pointer in the functor. This new implementation uses `thrust::tabulate` but we can add our own optimized kernel later if we need. Authors: - Louis Sugy (https://github.com/Nyrio) - Corey J. Nolet (https://github.com/cjnolet) Approvers: - Tamas Bela Feher (https://github.com/tfeher) - Corey J. Nolet (https://github.com/cjnolet) - Artem M. Chirkin (https://github.com/achirkin) URL: #1149
| IdxT n_rows, | ||
| LabelT* labels, | ||
| rmm::mr::device_memory_resource* mr) | ||
| inline std::enable_if_t<std::is_floating_point_v<MathT>> predict_core( |
There was a problem hiding this comment.
I hope that we can soon enable int8 types as well.
There was a problem hiding this comment.
int8 dataset with floating-point centroids, or all int8? The former means repeating the conversion or cast operation many times in the distance kernels, and for the latter, we have to evaluate if there could be a loss of quality in the clustering, or if learning could somehow get stuck?
There was a problem hiding this comment.
(I'm not sure) Aren't the distance kernels always memory-bound? If so, I don't think the conversion (which is usually a simple multiply + cast) would noticeably hurt performance.
| raft::device_matrix_view<const DataT, IndexT> X, | ||
| raft::device_matrix_view<MathT, IndexT> centroids, | ||
| MappingOpT mapping_op = raft::identity_op()) | ||
| { |
There was a problem hiding this comment.
If you require the mapping to be identity when DataT == MathT, consider adding a static assert:
static_assert(!std::is_same_v<MathT, DataT> || !std::is_same_v<MappingOpT, raft::identity_op>,
"Only the identity mapping is supported when MathT == DataT")There was a problem hiding this comment.
No, this would break the ANN methods that pass mapping<float> which behaves like the identity when the input type is float. We want an identity op, not the identity_op.
|
/merge |
No description provided.