Skip to content

Fix incorrect results from cumulative_sum and cumulative_prod with dtype=bool #2018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ndgrigorian merged 6 commits into from
Feb 28, 2025
Merged

Fix incorrect results from cumulative_sum and cumulative_prod with dtype=bool #2018

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e2d2b3f
Add missing kernels to arithmetic accumulators
ndgrigorian Feb 25, 2025
47239cd
Add tests based on gh-2017
ndgrigorian Feb 25, 2025
48b0e66
Add comment specifying need for work-around in accumulators
ndgrigorian Feb 26, 2025
c58d775
Reuse SYCL utils in workaround for logical operators in accumulators
ndgrigorian Feb 26, 2025
35d46d3
Alias conditional binary op type for cumulative_sum and cumulative_prod
ndgrigorian Feb 26, 2025
fa525a7
Add comment to needs_workaround in accumulators.hpp
ndgrigorian Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Reuse SYCL utils in workaround for logical operators in accumulators
  • Loading branch information
@ndgrigorian
ndgrigorian committed Feb 26, 2025
commit c58d77508a60ad6d4705d3c1cbf4559eef774d17
9 changes: 4 additions & 5 deletions dpctl/tensor/libtensor/include/kernels/accumulators.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ namespace kernels
namespace accumulators
{

namespace su_ns = dpctl::tensor::sycl_utils;

using dpctl::tensor::ssize_t;
using namespace dpctl::tensor::offset_utils;

Expand Down Expand Up @@ -87,9 +89,8 @@ template <typename srcTy, typename dstTy> struct CastTransformer
template <typename ScanOpT, typename T> struct needs_workaround
{
// work-around needed due to crash in JITing on CPU
Comment thread
ndgrigorian marked this conversation as resolved.
Outdated
static constexpr bool value =
std::is_same_v<ScanOpT, sycl::logical_or<T>> ||
std::is_same_v<ScanOpT, sycl::logical_and<T>>;
static constexpr bool value = su_ns::IsSyclLogicalAnd<T, ScanOpT>::value ||
su_ns::IsSyclLogicalOr<T, ScanOpT>::value;
};

template <typename BinOpT, typename T> struct can_use_inclusive_scan_over_group
Expand Down Expand Up @@ -153,8 +154,6 @@ template <typename T> class stack_strided_t

// Iterative cumulative summation

namespace su_ns = dpctl::tensor::sycl_utils;

using nwiT = std::uint32_t;

template <typename inputT,
Expand Down