Fixes boolean indexing for strided masks

[ndgrigorian]

This PR fixes some edge cases to boolean indexing that were not working as expected.

The cumulative sum used to get the count of set elements in the mask was misbehaving, especially for cases with negative strides (and most noticeably, the 1D case). This seems to have been caused by stride simplification can change the order elements are traversed in, which caused incorrect results.

Additionally, the offset parameter was also unused in when the contiguous kernel was called after simplification.

A new stride simplification method called compact_iteration_space was introduced to fix the problem.

• Have you provided a meaningful PR description?
• Have you added a test, reproducer or referred to an issue with a reproducer?
• Have you tested your changes locally for CPU and GPU devices?
• Have you made sure that new changes do not introduce compiler warnings?
• Have you checked performance impact of proposed changes?
• If this PR is a work in progress, are you opening the PR as a draft?

[github-actions]

View rendered docs @ https://intelpython.github.io/dpctl/pulls/1370/index.html

[coveralls]

coverage: 85.634%. remained the same when pulling 7a30569 on fix-boolean-indexing-cumsum into cf4660d on master.

[github-actions]

Array API standard conformance tests for dpctl=0.14.6dev4=py310ha25a700_21 ran successfully.
Passed: 916
Failed: 84
Skipped: 119

[oleksandr-pavlyk]

It was a mistake to use simplify_iteration_space_1 here. Not only will negative strides cause trouble, but so will F-contiguous mask arrays.

However I don't think removing simplification altogether is the right solution. We could use simplify_iteration_space_2 using C-contiguous strides for the shape of the input mask array for the second input.

[ndgrigorian]

It was a mistake to use simplify_iteration_space_1 here. Not only will negative strides cause trouble, but so will F-contiguous mask arrays.

However I don't think removing simplification altogether is the right solution. We could use simplify_iteration_space_2 using C-contiguous strides for the shape of the input mask array for the second input.

I've tested this out and it does also resolve the problem.

Would it be worthwhile then to write an abbreviated version of simplify_iteration_space, something like simplify_iteration_space_by_c_contig_strides, which removes logic that will never be true for a C-contig second input and doesn't have an unused strides vector and offset as byproducts?

[oleksandr-pavlyk]

Would it be worthwhile then to write an abbreviated version of simplify_iteration_space, something like simplify_iteration_space_by_c_contig_strides, which removes logic that will never be true for a C-contig second input and doesn't have an unused strides vector and offset as byproducts?

Yes, I was thinking about this as well. It is a good idea. I'd suggest naming it compact_iteration_space (restricted simplification).

[ndgrigorian]

@oleksandr-pavlyk
I've added an implementation for compact_iteration_space to the PR.

[github-actions]

Array API standard conformance tests for dpctl=0.14.6dev4=py310ha25a700_23 ran successfully.
Passed: 916
Failed: 84
Skipped: 119

[github-actions]

Array API standard conformance tests for dpctl=0.14.6dev4=py310ha25a700_30 ran successfully.
Passed: 916
Failed: 84
Skipped: 119

[github-actions]

Array API standard conformance tests for dpctl=0.14.6dev4=py310ha25a700_31 ran successfully.
Passed: 916
Failed: 84
Skipped: 119

[github-actions]

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

[github-actions]

Array API standard conformance tests for dpctl=0.14.6dev4=py310ha25a700_26 ran successfully.
Passed: 915
Failed: 85
Skipped: 119