Fix for non-contiguous strides

[viclafargue]

Fixes #4731

[github-actions]

This PR has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates. This PR will be labeled inactive-90d if there is no activity in the next 60 days.

[github-actions]

This PR has been labeled inactive-90d due to no recent activity in the past 90 days. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates.

[viclafargue]

rerun tests

[csadorf]

Since the newly created array should have a conformant CAI, could we just call the constructor again?

Suggested change

	else:
	cupy_data = cp.array(data, copy=True, order='C')
	self._ptr = cupy_data.data.ptr
	self._owner = cupy_data if cupy_data.flags.owndata \
	else data
	self.order = 'C'
	self.strides = cupy_data.strides
	else:
	cupy_data = cp.array(data, copy=True, order='C')
	super().__init__(data=cupy_data)

[csadorf]

I don't think there is a need for these copies and you can also just use the sliced view of those arrays directly.

[csadorf]

Suggested change

	raise ValueError('Order must be "F" or "C". ')
	raise ValueError('Order must be "F" or "C".')

[csadorf]

We could consider to use a combination of itertools.accumulate() and operator.mul() to compute the strides a bit more succinctly:

from itertools import accumulate
from operator import mul


f_strides = list(accumulate(shape[:-1], func=mul, initial=item_size))
c_strides = list(accumulate(shape[:0:-1], func=mul, initial=item_size))[::-1]

Edit: If you like the suggestion, I can run some benchmarks to ensure that this isn't slower by any chance.

[viclafargue]

Thanks for the review. Sure that could be interesting :)

[csadorf]

Looks like loops are the fastest, however I was able to apply a micro optimization that is a bit faster (see below):

Loops:              100.00%
Loops (alt):        94.73%
With accumulate:    132.87%

I ran this a few times and results appear pretty consistent.

Benchmark code

from itertools import accumulate
from operator import mul


def compute_strides(shape, item_size):
    tuple(accumulate(shape[:-1], func=mul, initial=item_size))
    tuple(accumulate(shape[:0:-1], func=mul, initial=item_size))[::-1]


def compute_strides_loops(shape, item_size):

    strides = [item_size]
    for dim_size in shape[:-1]:
        strides.append(dim_size * strides[-1])
    tuple(strides)

    strides = [item_size]
    shape = shape[::-1]
    for dim_size in shape[:-1]:
        strides.append(dim_size * strides[-1])
    tuple(strides[::-1])


def compute_strides_loops_alternative(shape, item_size):

    strides = [item_size]
    for dim_size in shape[:-1]:
        strides.append(dim_size * strides[-1])
    tuple(strides)

    strides = [item_size]
    for dim_size in shape[:0:-1]:
        strides.append(dim_size * strides[-1])
    tuple(strides[::-1])


if __name__ == "__main__":
    from timeit import timeit

    result_w_loops = timeit(
        "compute_strides((2, 3), 8)",
        setup="from benchmark_strides import compute_strides_loops as compute_strides",
    )

    result_w_loops_alt = timeit(
        "compute_strides((2, 3), 8)",
        setup="from benchmark_strides import compute_strides_loops_alternative as compute_strides",
    )

    result_w_accumulate = timeit(
        "compute_strides((2, 3), 8)",
        setup="from benchmark_strides import compute_strides",
    )

    print(f"Loops:              {result_w_loops / result_w_loops:.2%}")
    print(f"Loops (alt):        {result_w_loops_alt / result_w_loops:.2%}")
    print(f"With accumulate:    {result_w_accumulate / result_w_loops:.2%}")

Micro-optimization:

    # F-order
    strides = [item_size]
    for dim_size in shape[:-1]:
        strides.append(dim_size * strides[-1])
    tuple(strides)

   # C-order
    strides = [item_size]
    for dim_size in shape[:0:-1]:
        strides.append(dim_size * strides[-1])
    tuple(strides[::-1])

[dantegd]

@gpucibot merge