v923z / micropython-ulab

a numpy-like fast vector module for micropython, circuitpython, and their derivatives
https://micropython-ulab.readthedocs.io/en/latest
MIT License
419 stars 115 forks source link

[BUG] ulab.numpy.roll doesn't copy over data when distance is 0 #686

Closed dcooperdalrymple closed 1 month ago

dcooperdalrymple commented 1 month ago

Describe the bug A program I am working on performs a call to ulab.numpy.roll to shift around an array by an varying amount of distance. I noticed that when the distance argument is set to 0, the function returns an array of the correct length but with all 0s. Quickly scanning over the code for this in code/numpy/numerical.c, my guess is that L1224 is never run to copy over the results.

https://github.com/v923z/micropython-ulab/blob/eacb0c9af47f85f5d4864b721c3b28661364e8e3/code/numpy/numerical.c#L1223-L1225

ulab version: 6.4.1-2D

To Reproduce

import ulab.numpy as np
print(data := np.arange(10))
print(np.roll(data, 1))
print(np.roll(data, 0))

Output:

array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9], dtype=int16)
array([9, 0, 1, 2, 3, 4, 5, 6, 7, 8], dtype=int16)
array([0, 0, 0, 0, 0, 0, 0, 0, 0, 0], dtype=int16)

Describe the steps to reproduce the behavior.

Expected behavior I would expect the output of np.roll(data, 0) to simply pass through the data. If the axis is provided, that may have different results, but I am not certain of the expected behavior in that case.

I think a decent enough solution would be to detect if the distance/shift argument is 0 and pass the output through early.

Changing numerical.c#L1223 to if(--counter <= 0) { may also work, but I suspect that it would have the same effect as if distance were equal to 1, which would not be desired.

v923z commented 1 month ago

Thanks for pointing this out!

v923z commented 1 month ago

Fixed in https://github.com/v923z/micropython-ulab/pull/687