You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "andishgar (via GitHub)" <gi...@apache.org> on 2025/06/11 07:42:07 UTC

[I] [C++] Enable EqualOptions::atol_ for arrow::Tensor and arrow::SparseTensor, and EqualOptions::signed_zero_ for arrow::SparseTensor [arrow]

andishgar opened a new issue, #46776:
URL: https://github.com/apache/arrow/issues/46776

   ### Describe the bug, including details regarding any error messages, version, and platform.
   
   As [mentioned here](https://github.com/apache/arrow/issues/46767#issuecomment-2959468747), `arrow::EqualOptions::atol_` is not applied when comparing `arrow::Tensor` and` arrow::SparseTensor`.
   
   Additionally, I found that signed zero is not handled correctly in `arrow::SparseTensor`. For example, the following assertion unexpectedly passes:
   
   ```c++
   TEST(Tensor,FloatComparasion) {
     std::vector<double> v1 = {+0.0};
     std::vector<double> v2 = {-0.0};
     std::shared_ptr<Buffer> b1 = Buffer::Wrap(v1);
     std::shared_ptr<Buffer> b2 = Buffer::Wrap(v2);
     NumericTensor<DoubleType> t1(b1, {1, 1});
     NumericTensor<DoubleType> t2(b2, {1, 1});
     auto st1 = SparseCSCMatrix::Make(t1).ValueOrDie();
     auto st2 = SparseCSCMatrix::Make(t2).ValueOrDie();
     ASSERT_FALSE(st1->Equals(*st2, EqualOptions::Defaults().signed_zeros_equal(true)));
     ASSERT_FALSE(st1->Equals(*st2, EqualOptions::Defaults().signed_zeros_equal(false)));
   }
   ``` 
   
   ### Component(s)
   
   C++


-- 
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: issues-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [C++] Enable EqualOptions::atol_ for arrow::Tensor and arrow::SparseTensor, and EqualOptions::signed_zero_ for arrow::SparseTensor [arrow]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on issue #46776:
URL: https://github.com/apache/arrow/issues/46776#issuecomment-2961730988

   I don't think anyone is planning to work on this. @rok do you?


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [C++] Enable EqualOptions::atol_ for arrow::Tensor and arrow::SparseTensor, and EqualOptions::signed_zero_ for arrow::SparseTensor [arrow]

Posted by "rok (via GitHub)" <gi...@apache.org>.
rok commented on issue #46776:
URL: https://github.com/apache/arrow/issues/46776#issuecomment-2961760715

   I'm not planning to work on this in the short term.
   As a workaround you can use [SparseTensor.ToTensor](https://arrow.apache.org/docs/cpp/api/tensor.html#_CPPv4N5arrow12SparseTensorE) to convert your sparse tensor to a dense one and then compare the two dense tensors.
   The signed zeros issue would be nice to fix.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [C++] Enable EqualOptions::atol_ for arrow::Tensor and arrow::SparseTensor, and EqualOptions::signed_zero_ for arrow::SparseTensor [arrow]

Posted by "andishgar (via GitHub)" <gi...@apache.org>.
andishgar commented on issue #46776:
URL: https://github.com/apache/arrow/issues/46776#issuecomment-2961592335

   @pitrou Does anyone plan to work on this issue? I'm not familiar with the arrow::Tensor and arrow::SparseTensor APIs, so it will take me some time to understand them and write tests for common and edge cases


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [C++] Enable EqualOptions::atol_ for arrow::Tensor and arrow::SparseTensor, and EqualOptions::signed_zero_ for arrow::SparseTensor [arrow]

Posted by "andishgar (via GitHub)" <gi...@apache.org>.
andishgar commented on issue #46776:
URL: https://github.com/apache/arrow/issues/46776#issuecomment-3018078044

   @pitrou Three days after reporting these issues, I unfortunately encountered a problem that prevented me from doing any coding activity for two weeks. I will probably not be able to work on this issue before August 30th. So I’m totally fine if someone else wants to take it over.
   By the way, I had a chance to look at the arrow::Tensor API, and I think there are several things worth mentioning.
   
   1- `ARROW_PREDICT_FALSE` appears to be appropriate for the following code.
   https://github.com/apache/arrow/blob/f8cd17c0651e4886a08b2664ec8e0a0fff09eaa2/cpp/src/arrow/tensor.cc#L57
   
   2-The following assertions should use` ASSERT_FLOAT_EQ` instead.
   https://github.com/apache/arrow/blob/953947a78157090ff94b14bc027cbcf520a5b2a6/cpp/src/arrow/tensor_test.cc#L708-L713
   
   https://github.com/apache/arrow/blob/953947a78157090ff94b14bc027cbcf520a5b2a6/cpp/src/arrow/tensor_test.cc#L747-L752
   
   3- The code below could be split into two separate paths for contiguous and non-contiguous tensors, similar to what's done in [counting zeros](https://github.com/apache/arrow/blob/953947a78157090ff94b14bc027cbcf520a5b2a6/cpp/src/arrow/tensor.cc#L513-L520) .
   
   https://github.com/apache/arrow/blob/953947a78157090ff94b14bc027cbcf520a5b2a6/cpp/src/arrow/compare.cc#L1248-L1292
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [C++] Enable EqualOptions::atol_ for arrow::Tensor and arrow::SparseTensor, and EqualOptions::signed_zero_ for arrow::SparseTensor [arrow]

Posted by "andishgar (via GitHub)" <gi...@apache.org>.
andishgar commented on issue #46776:
URL: https://github.com/apache/arrow/issues/46776#issuecomment-2963983945

   @rok Thank you for your guidance.
   
   @pitrou I will inform you whether I decide to take on the issue or not.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [C++] Enable EqualOptions::atol_ for arrow::Tensor and arrow::SparseTensor, and EqualOptions::signed_zero_ for arrow::SparseTensor [arrow]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on issue #46776:
URL: https://github.com/apache/arrow/issues/46776#issuecomment-3018092634

   > [@pitrou](https://github.com/pitrou) Three days after reporting these issues, I unfortunately encountered a problem that prevented me from doing any coding activity for two weeks. I will probably not be able to work on this issue before August 30th. So I’m totally fine if someone else wants to take it over.
   
   I think it's ok, there's no urgency anyway. Thank you for your contributions!


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org