francof2a / fxpmath

A python library for fractional fixed-point (base 2) arithmetic and binary manipulation with Numpy compatibility.
MIT License
183 stars 21 forks source link

Unexpected behavior in assignment of fixed point variables initialized with a 2D list #62

Closed lucacutrignelli closed 2 years ago

lucacutrignelli commented 2 years ago

The following code produce a result that is not what I expect using 2D lists. Trying to update an element of my 2D list does not succeed Equivalent code works correctly in case of 1D list

Is this the expected behavior? In case it is, what is the suggested way of coding the intended behavior?

>>> y = Fxp(dtype='fxp-s6/2')
>>> y([[1.0,0.25,0.5],[0.25,0.5,0.25]])

>>> y[0][0] = y[0][0]+1.0
>>> y[0][0]
fxp-s6/2(1.0)

The operation is correctly performed, but the assignment is not

>>> y[0][0]+1.0
fxp-s6/2(2.0)
francof2a commented 2 years ago

Fxp returns a new Fxp when you do an indexing. So, doing y[i][j] you get a new Fxp with y[i] and then index the last one with j.

When you're getting values there are no problems because you're getting a new object with the value you need, but when you're setting values, concatenated indices will set values over new object and not over original one.

y[1][2] = 3.0
# it's equivalent to:
y_new = y[1]
y_new[2] = 3.0

You should use multiple indexing at the same time (all indices between [ ]), then:

y[0,0] = y[0,0] + 1.0

should work!

lucacutrignelli commented 2 years ago

I appreciate very much the prompt and clear explanation!

Indeed your suggestion works and it's the best practice also for numpy arrays as I just learned on https://numpy.org/doc/stable/user/basics.indexing.html

If we compare the behavior of Fxp with numpy arrays, the latter operate differently and the copied object is pointing to the same memory contents. So for a numpy array, creating y_new and assigning a new value would also change the original y

This discrepancy in behavior might be confusing for users, so if technically possible you might evaluate whether to change this in future. Nonetheless your comment solved my issue and made me learn something new on Fxp and numpy arrays indexing. Thanks!

francof2a commented 2 years ago

I'm glad I could help you!

You're right about functionality. It's in the development plan to implement the view functionality instead of copy, but it is no quite straight forward. Thanks for do the observation!

francof2a commented 2 years ago

I close the issue considering it solved in version 0.4.8.

y[0][0] = y[0][0] + 1.0

Now it's working!