v923z / micropython-ulab

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

[FEATURE REQUEST] Implement floor divide #590

Closed Friik90 closed 1 year ago

Friik90 commented 1 year ago

Describe the bug According to the documentation, operations on two ndarrays of the same dtype preserve their dtype. However, this is not the case when performing division, which always results in float32.

I'm on ulab version 6.0.7-2D-c.

To Reproduce

from ulab import numpy as np

arr1 = np.ones((3,3), dtype=np.uint8)
arr2 = np.ones((3,3), dtype=np.uint8)
res = arr1 / arr2
print(res)

Expected behavior I expected the dtype of res to still be uint8, but it's float32 instead.

v923z commented 1 year ago

That is actually the numpy behaviour, too:

>>> import numpy as np
>>> a = np.array([1, 2, 3], dtype=np.uint8)
>>> a/a
array([1., 1., 1.])

However, if you want to keep the dtype, you should use true divide

>>> import numpy as np
>>> a = np.array([1, 2, 3], dtype=np.uint8)
>>> a // a
array([1, 1, 1], dtype=uint8)

So, I believe, the implementation is correct, but the documentation is misleading.

Friik90 commented 1 year ago

I could live with that, but then the problem is that floor division apparently isn't implemented right:

>>> from ulab import numpy as np
>>> arr1 = np.array([1,2,3], dtype=np.uint8)
>>> res = arr1 // arr1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported types for __floordiv__: 'ndarray', 'ndarray'

The same code works with regular NumPy, as in your example.

v923z commented 1 year ago

Floor division is not implemented at all. That's what your traceback is actually saying. Do you need it?

Friik90 commented 1 year ago

Desperately actually :laughing:. I have to perform some fixed-point arithmetic, so either floor division or bit-shift operation is required at some point. Iterating over the array and performing this operation element by element is too slow in my case.

v923z commented 1 year ago

I'll try to whip up an implementation by tomorrow night. However, I might ask you to write a small test script.

Friik90 commented 1 year ago

Sure, I can do that. That'd be awesome.

v923z commented 1 year ago

@Friik90 Do you think you could check out the https://github.com/v923z/micropython-ulab/tree/floordiv branch, and see if that does what it's supposed to do?

v923z commented 1 year ago

@Friik90 Do you think you could create a test script for https://github.com/v923z/micropython-ulab/pull/593? I would then merge this into master.

matemaciek commented 1 year ago

One more user needing floor divide here. It's good that I checked existing issues before starting crafting my own // operator (-:

matemaciek commented 1 year ago

Tested it with circuitpython, getting some unexpected results:

>>> from ulab import numpy as np
>>> np.array([1,2,3],dtype=np.int8) // 2
array([0, 0, 0], dtype=int8)
>>> np.array([1,2],dtype=np.int8) // 2
array([0, 0], dtype=int8)
>>> np.array([2],dtype=np.int8) // 2
array([1], dtype=int8)
>>> np.array([2,3],dtype=np.int8) // 2
array([1, 1], dtype=int8)
>>> np.array([1,2],dtype=np.int8) // 2
array([0, 0], dtype=int8)
matemaciek commented 1 year ago

I'll add it to test cases and see whether it can be replicated there

matemaciek commented 1 year ago

Added test cases and fixed bug above: https://github.com/v923z/micropython-ulab/pull/599

I'll try to add in-place floor div also

matemaciek commented 1 year ago

...nope, I give up on adding in-place version. I was hoping that the in-place versions of operators reuse code from binary operators, but they expand to C in-place operators, and there is no //= in C )-:

v923z commented 1 year ago

Implemented in https://github.com/v923z/micropython-ulab/pull/599, https://github.com/v923z/micropython-ulab/pull/593.

v923z commented 1 year ago

...nope, I give up on adding in-place version. I was hoping that the in-place versions of operators reuse code from binary operators, but they expand to C in-place operators, and there is no //= in C )-:

We could actually think about consolidating the binary operator code and the in-place versions, though it's not entirely trivial: it's not enough to just pass a different pointer to a function, because depending on what your operators are, you'll have to iterate over them in different fashions.

matemaciek commented 1 year ago

Also >>= and <<= are very tempting to add, and should be easier than //=.

v923z commented 1 year ago

Is it something that's actually used? We can't just add everything that's easy to implement...

matemaciek commented 1 year ago

Yeah, I was actually looking for them, when trying too speed up computations that does not need float precision. But either way it'd need some benchmarking what's faster: one float operation (division) or three int operations (shift left, floor divide, shift right), as just an overhead of jumping back to python might outweigh the faster computations.

Anyway thanks to your library I already sped up my thermal camera interpolation significantly - keeping ~10fps, but going from 7 midpoints to 34 (-: