Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pow() of Fxp unsigned values does not work #17

Closed
arnfol opened this issue Nov 10, 2020 · 4 comments
Closed

pow() of Fxp unsigned values does not work #17

arnfol opened this issue Nov 10, 2020 · 4 comments

Comments

@arnfol
Copy link

arnfol commented Nov 10, 2020

Hi! First of all, thanks for this module, great work!

I've noticed, that Fxp does not support the power of unsigned values. The following example

a = Fxp(15, signed=False)
b = a ** 2

Generates traceback:

Traceback (most recent call last):
  File "dpd_model.py", line 16, in <module>
    b = a ** 2
  File "/home/arnfol/anaconda3/lib/python3.8/site-packages/fxpmath/objects.py", line 608, in __pow__
    y = Fxp(self.get_val() ** n, signed=self.signed or n.signed, n_word=n_word, n_frac=n_frac)
AttributeError: 'int' object has no attribute 'signed'

Which is, obviously, because integer n does not have a signed field :) I think that or n.signed is not needed here, because the sign of the number in pow does not affect the resulting sign.

I also suppose that the __pow__ method does not generate correct n_word bit size for signed values. Rigth now it multiplies sign bit by n, which is not needed. I assume it should be like:

def __pow__(self, n):
    n_intg = self.n_word - self.n_frac - 1 if self.signed else self.n_word - self.n_frac
    n_frac = self.n_frac * n
    n_word = n_intg * n + n_frac + 1 if self.signed else n_intg * n + n_frac

    y = Fxp(self.get_val() ** n, signed=self.signed, n_word=n_word, n_frac=n_frac)
    return y

But I haven't tested it yet.

@arnfol arnfol changed the title pow() of Fxp signed values does not work pow() of Fxp unsigned values does not work Nov 10, 2020
@arnfol
Copy link
Author

arnfol commented Nov 10, 2020

On the second thought, the resulting sign depends on n//2. So we can get rid of sign bit if n//2 == 0, but in this case return object could be signed or unsigned, depending on n. Could it be a problem?

@francof2a
Copy link
Owner

francof2a commented Nov 11, 2020

Hi,
Thank you very much. I'm glad this module is useful for you and sorry about bug, but it is immature yet.

About the signed field in n, you are right, it's a bug considering that n is a Fxp object.

You can get the integer size of Fxp using n_int property, so, you don't need to take care about sign :) (just info)
But, you're right about in case of signed n_word = n*(n_int + n_frac) + 1 and not n_word = n*n_word

All of these makes me thing that this method is not supporting float or negative exponents (n).


About your second thought, it is not convenient return a signed or not signed object depending its val. The philosophy I choose for fxpmath is to maintain the signed type of Fxp regardless its value.

I imagine that all of these changes will be included in next release (soon)

@arnfol
Copy link
Author

arnfol commented Nov 12, 2020

Oh, I've noticed that my fix #18 also does not consider the cases there power n is negative. As I understand in those cases only the fractional part should be extended.

francof2a added a commit that referenced this issue Nov 12, 2020
* hex representations support padding.
* fix bug with 'b' in hex string (issue #15).
* fix inaccuracy word in status (issue #16).
* fix __pow__ method (issue #17).
* fix bug that only store imaginary part in complex assignement by indexing (issue #19).
@francof2a
Copy link
Owner

well, I update the repo and release.
This issue should be solved. I change a little bit the solution taking into account other cases.

Obs: please check in your solution if the following case works:

x = Fxp(-16)
y = x ** 2
assert y() == 256

I think one bit is missing in your solution.

Tell me please if v0.3.8 is working for you.
Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants