pitrou commented on code in PR #47586:
URL: https://github.com/apache/arrow/pull/47586#discussion_r2413847562
##########
cpp/src/arrow/tensor/csf_converter.cc:
##########
@@ -157,6 +235,8 @@ class SparseCSFTensorConverter : private
SparseTensorConverterMixin {
ARROW_ASSIGN_OR_RAISE(
sparse_index, SparseCSFIndex::Make(index_value_type_, indices_shapes,
axis_order,
indptr_buffers, indices_buffers));
+ DCHECK_OK((ValidateSparseTensorCSFCreation<ValueType,
IndexType>(*sparse_index, *data,
Review Comment:
Same question here.
##########
cpp/src/arrow/tensor/coo_converter.cc:
##########
@@ -220,7 +265,8 @@ class SparseCOOTensorConverter : private
SparseTensorConverterMixin {
indices_shape, indices_strides);
ARROW_ASSIGN_OR_RAISE(sparse_index, SparseCOOIndex::Make(coords, true));
data = std::move(values_buffer);
-
+ DCHECK_OK((ValidateSparseCooTensorCreation<ValueType,
IndexType>(*sparse_index, *data,
Review Comment:
Hmm, so this is only executed in debug mode? Note this is a `O(n)` function
with n potentially large, so I'm not sure we really want to run this every time
a tensor is converted.
Perhaps we can make this a `Validate` method on the sparse tensor class
instead?
##########
cpp/src/arrow/tensor.h:
##########
@@ -55,6 +55,13 @@ constexpr bool is_tensor_supported(Type::type type_id) {
namespace internal {
+// TODO(GH-47578): Enable HalfFloatType
+template <typename ValueDataType>
+inline bool is_not_zero(typename ValueDataType::c_type value) {
+ typename ValueDataType::c_type zero = 0;
+ return value != zero;
Review Comment:
Ok, so this returns `false` if `value` is NaN? Is that desirable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
