FixedPointNumbers.jl icon indicating copy to clipboard operation
FixedPointNumbers.jl copied to clipboard

Wrong printing of `Fixed` numbers

Open PatrickHaecker opened this issue 1 year ago • 5 comments

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.

PatrickHaecker avatar Aug 01 '24 12:08 PatrickHaecker

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.

kimikage avatar Aug 02 '24 17:08 kimikage

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?

PatrickHaecker avatar Aug 03 '24 04:08 PatrickHaecker

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.

kimikage avatar Aug 03 '24 07:08 kimikage

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?

PatrickHaecker avatar Aug 05 '24 05:08 PatrickHaecker

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.

kimikage avatar Aug 05 '24 14:08 kimikage