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

Wrong printing of Fixed numbers #307

Open
PatrickHaecker opened this issue Aug 1, 2024 · 5 comments
Open

Wrong printing of Fixed numbers #307

PatrickHaecker opened this issue Aug 1, 2024 · 5 comments

Comments

@PatrickHaecker
Copy link

PatrickHaecker commented Aug 1, 2024

Either I have totally misunderstood the documentation or printing of Fixed numbers is wrong.

julia> Fixed{Int8,2}(0.25)
0.2Q5f2

julia> Fixed{Int8,3}(0.125)
0.12Q4f3

Looking into FixedPointNumbers.jl/show leads to

digits=ceil(Int, f * log10_2)

which is wrong, as for 2^-f the number of decimal fraction digits scales linearly and not logarithmic with f's number of binary fraction digits, as you can see here

julia> 2.0.^(-1:-1:-10)
10-element Vector{Float64}:
 0.5
 0.25
 0.125
 0.0625
 0.03125
 0.015625
 0.0078125
 0.00390625
 0.001953125
 0.0009765625

Consequently this can be fixed by using digits=f.

@kimikage
Copy link
Collaborator

kimikage commented Aug 2, 2024

The printing may not be accurate, but it is not wrong.
Similar imprecision is very often seen with floating-point numbers.

julia> 0.1f0
0.1f0

julia> big(0.1f0)
0.100000001490116119384765625

This inexact printing confuses many newbies, but exact printing is also not what everyone is looking for.

This package also provides Normed. It is generally impossible to print a Normed number in decimal numbers with finite digits.

Personally, I think an additional digit would be nice. However, it is not essential in round-tripness and is a matter of preference.

@PatrickHaecker
Copy link
Author

That is an interesting perspective. Thanks for sharing it.

When comparing fixed-point numbers with floating-point numbers I agree that this is only a matter of accuracy.

I also understand that normed numbers (as in Normed) are this is a separate discussion

In my mental model fixed-point numbers (as in Fixed) are much more similar to integers than to floating-point numbers. Therefore, for me

julia> Fixed{Int8,2}(0.25)
0.2Q5f2

is the same as if we'd have

julia> UInt8(25)
20

I know this, because I spent like 10 minutes reading through the documentation again and again, checking my one-liner for bugs and questioning my understanding of fixed point numbers until I finally looked at the source and saw that the printing does not print out the correct (identical to the internal representation) result.

Would you accept a PR with this change for Fixed?

@kimikage
Copy link
Collaborator

kimikage commented Aug 3, 2024

As noted above, there is a distinct difference there in terms of round-tripability.

julia> Fixed{Int8,2}(0.25) == 0.2Q5f2
true

julia> UInt8(25) == 20
false

The direct solution to the time wasting problem you have experienced would be to add documentation.

I am in favor of setting a lower bound for digits or adding just one digit for Fixed. However, exact representation in decimal numbers is redundant, both from a practical standpoint and in terms of the entropy.

It would be nice if julia or the REPL had a mechanism to control the number of digits displayed, but as far as I know, they do not.

@PatrickHaecker
Copy link
Author

Ah, thanks, I only now understood what you meant with round-tripability.

However, exact representation in decimal numbers is redundant, both from a practical standpoint and in terms of the entropy.

I agree that it is not necessary in terms of the entropy. However, printing out numbers is generally not fully entropy-optimized, otherwise we wouldn't print binary numbers in decimal (the ceil in the relevant code). Instead, the printing should be a user service and doing all the fixed point math in your head, although possible, is cumbersome.

It would be nice if julia or the REPL had a mechanism to control the number of digits displayed, but as far as I know, they do not.

I think the :compact key essentially does this:

:compact: Boolean specifying that values should be printed more compactly, e.g. that numbers should be printed with fewer digits.

So I guess a compromise could be to print the entropy-optimized, round-trippable number when :compact == true and the correct decimal number when :compact == false. Additionally, I would add documentation about the issue.

Would that work for you?

@kimikage
Copy link
Collaborator

kimikage commented Aug 5, 2024

Although FPN v0.9 has been under development for a very long time, it does support the :compact key (in a somewhat different way). (cf. PR #189)

This package is not mine, and I would rather that the development of this package evolve independent of me.
For personal reasons, I am once again greatly limited in the time and stamina I can allocate to the development.

If you are willing to take responsibility for the change (i.e., help us resolve some remaining tasks and release the change as v0.9 or backport it to v0.8), I have no reason to oppose your proposal.

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