Compact systematic display of values #51

timholy · 2016-09-15T22:31:02Z

Note: this proposal has been updated, please skip ahead to #51 (comment)

This introduces value printing of the following form:

julia> UFixed{UInt16, 11}(0.4)
0.4001U₁₁¹⁶

julia> Fixed{Int32,5}(0.4)
0.41F₅³²

Moreover, for UFixed8, UFixed10, ..., the same sequence of characters is interpreted as a constant that allows values to be reconstructed:

julia> x = UFixed10(0.4)
0.3998U₁₀¹⁶

julia> x == 0.3998U₁₀¹⁶
true

This has pros and cons:

Printing is systematic and comprehensive; any FixedPoint value has a self-explanatory compact representation
The con is that we might need "generated constants," i.e., we lack a way to have these parse automatically for arbitrary choices of T and f. Or some way of modifying julia's parser.

timholy · 2016-09-15T22:34:48Z

Maybe instead of F we should use Q: https://en.wikipedia.org/wiki/Fixed-point_arithmetic#Notation

timholy · 2016-09-15T23:48:14Z

It occurs to me that if we generated all typealiases (and yes, now I think they can be typealiases rather than consts), it would "only" be approx 256 exports. That's a lot, but perhaps not so many so as to make it untenable.

vchuravy · 2016-09-15T23:53:36Z

Can they be type aliases and be in the suffix form? I was thinking of having a major that would generate the const in the users namespace, but I like the idea of just generating most of them ;) (What about Uint128 based ones ;))

timholy · 2016-09-16T02:58:47Z

OK, an update here (I think this implementation is "complete"). The proposal is to print Fixed values like this:

julia> Fixed{Int16,7}(1.23)
1.227Q⁸₇

Here we're using the Q notation, where 8 is the number of magnitude bits and 7 the number of fractional bits. Note that 8+7=15, leaving one bit for the sign. I am not printing this as Q8.7, because we want the output representation to parse back to the actual value, and the . would prevent that from happening. So I introduced the super/subscripts to indicate the two numbers; I put the magnitude first and as a superscript, since it reflects the "bigger" (higher) part and also comes first.

Likewise, we'd print UFixed values like this:

julia> UFixed{UInt16,7}(1.23)
1.228U⁹₇

9 is the number of magnitude bits, and again 7 is the number of fractional bits; the UFixed numbers don't have a sign bit, so they'll always add to a twos-complement number. We can't use Q⁹₇ here because the normalization is different (2^f-1 rather than 2^f, see the README). The U is therefore something I just made up, but it seems reasonable.

In this implementation, these are typealiases for the actual type, and we export all possible typealiases up to 64 bits. I also (somewhat unconventionally) define * for a Real value and any of the Fixed/UFixed types, and use that as a constructor. This ensures that copy/paste of output value(s) reconstructs the same value(s).

Another consequence is that length(names(FixedPointNumbers)) = 267, which is a considerable number of exports.

On balance I like this proposal; the alternative is to print values as UFixed{UInt16,7}(1.228), which is quite a mouthful. One likely concern is how well the super/subscripts will render on different terminals.

vchuravy · 2016-09-16T04:42:54Z

In general I really like this, but the super and subscripts can be hard to read. I program in a font-size of 14, but even then it is not clearly visible. But +1 for more compact printing, but this will also require a prominent explanation in the readme.

timholy · 2016-09-16T11:02:18Z

Any good ideas for how to handle this without using super/subscripts? At the cost of an extra character, we could indicate the split with an additional unicode character. Perhaps U9⊚7 (http://www.fileformat.info/info/unicode/char/229a/index.htm) or U9⊘7 (http://www.fileformat.info/info/unicode/char/2298/index.htm). The choice seems pretty arbitrary to me, except of course for making sure we don't misuse something that already means something else.

bjarthur · 2016-09-16T11:20:41Z

you didn't entirely make up the U: "In addition, the letter U can be prefixed to the Q to indicate an unsigned value, such as UQ1.15, indicating values from 0.0 to +1.99997." (from the wiki article you linked).

bjarthur · 2016-09-16T11:32:06Z

what are the consequences of exporting so many names?

bjarthur · 2016-09-16T11:38:56Z

there is a convention to use B for byte (8 bits), W for word (16), D for double (32) and Q for quad (64). so one alternative to super/sub-scripts would be to say use B3 for UFixed{UInt8,3} and D7 for UFixed{UInt32,7}. for the signed counterparts one could add an S prefix: SQ11 is Fixed{Int64,11}. just thinking out loud.

timholy · 2016-09-16T13:55:02Z

you didn't entirely make up the U: "In addition, the letter U can be prefixed to the Q to indicate an unsigned value, such as UQ1.15, indicating values from 0.0 to +1.99997." (from the wiki article you linked).

True, but the normalization is still different. What they mean by UQ1.15 would be the equivalent of Fixed{UInt16,15} rather than UFixed{UInt16,15}. (We don't support the former yet, since Fixed requires Signed, but there's no reason we couldn't drop that requirement, except for the fear of adding confusion.) So UQ1.15 has a maximum value of

julia> 0xffff/0x8000
1.999969482421875

whereas

julia> typemax(UFixed{UInt16,15})
2.00003U¹₁₅

is equal to 0xffff/0x7fff.

In case this confuses you, the easy way to remember why we have this distinction is like this:

julia> 0xff/256  # This is typemax(UQ0.8)
0.99609375

and so convert(UQ0.8, 1.0) would throw an InexactError (it's the next "integer" up, so it doesn't round down to a valid number). For floating point images, "saturated" = 1.0, so for seamless interconversion between ("valid") floating-point and fixed-point we require that 1.0 lies within the domain of the fixed-point number type.

timholy · 2016-09-16T13:57:10Z

The B, W, D, Q suggestion is quite interesting. I was trying to pick something close to "standard" notation, but it's clear that standards don't cover all of our needs, so we are going to have to strike out on our own somewhat.

timholy · 2016-09-16T13:58:26Z

what are the consequences of exporting so many names?

I don't think it's terrible. It just seems like a lot for a small package. By comparison, length(names(Base)) = 1392 right now.

bjarthur · 2016-09-16T15:00:30Z

if you decide to go with BWDQ, i'd suggest instead to prepend with U for unsigned instead of S for signed, to mirror UInt and Int.

vchuravy · 2016-09-16T15:10:38Z

I would reserve UQ for the moment when we have Fixed{<:Unsigned}. I don't particularly care for W, D, Q since word means something different depending on the underlying architecture.
An alternative could be more to deviate from the default Q notation and use Q16₁₅. Where Q16 is Fixed{Int16} so this would be closer to how base works.

@timholy we could do a soft-rename in this case. Let's start using N for UFixed in anticipation for the rename to Norm/Normed (and I promise that I will get around to that soon.)

bjarthur · 2016-09-16T15:54:46Z

another alternative requiring little explanation would be UInt16F3==UFixed{UInt16,3}
and Int16F7 == Fixed{Int16,7}

timholy · 2016-09-16T17:53:50Z

@vchuravy, In that case, N definitely sounds like a good idea. I doubt there would ever be a "normalized signed" variant, because the existing Fixed{T<:Signed} types are already normalized in the negative direction (just not in the positive direction). So I don't think we need to have U in the name. (But chime in if you disagree.)

@bjarthur, I agree that being more explicit has some merits, but it's still pretty long compared to some of the choices we've been discussing. N9x7 (where x is to be determined) is only 4 characters, compared to 8 for UInt16F7.

One thing your example does convince me of, though, is that maybe it wouldn't be so terrible to use a roman character for x. I've been considering unicode mostly to avoid conflicting with common variable names, but really, how many programs use variables with names like what we're considering?

So perhaps N9f7 and Q8f7? The f is reminiscent of the 3.2f0 notation for constructing Float32s. One potential downside is that in f0, the 0 seemingly doesn't have a meaning (or at least, I'm not aware of one), whereas for us it would be very meaningful (and rarely, could indeed be 0). So there's some potential for confusion.

vchuravy · 2016-09-16T18:15:54Z

No, I agree there won't be a signed variant of Normed. (Normalised???), so N should be sufficient.

I am having a hard time deciding between:

N16f8 == UFixed{UInt16, 8}
Q8f4 == Fixed{Int8, 4}
UQ16f10 == Fixed{UInt16,10}

and:

N8f8 == UFixed{UInt16, 8}
Q3f4 == Fixed{Int8, 4}
UQ6f10 == Fixed{UInt16, 10}

I find the first one easier to read, but the second one is more succinct (but requires more mental effort to parse)

timholy · 2016-09-16T18:25:18Z

I also initially went with the equivalent of Q8f4, and still find that somewhat more intuitive. But one fairly big problem is that's inconsistent with the conventional notation: https://en.wikipedia.org/wiki/Fixed-point_arithmetic#Notation. (I know they don't use f, but perhaps that's a small detail.)

Also, I think you have the opposite meaning of the number after the f that I've been using. I'm literally copying the value of f (the type parameter) in there. I think that's consistent with https://en.wikipedia.org/wiki/Fixed-point_arithmetic#Notation? "m = number of magnitude or integer bits, f = number of fraction bits" So I think their f is the same as our f. Their m = b - f - s.

vchuravy · 2016-09-16T18:33:17Z

ah I just made a mistake for UQ5f10. Yeah it is a conflict between the Julia convention of giving the full nbits of a type. and the fixed-point arithmetic notation.

timholy · 2016-09-16T18:37:55Z

I took the liberty of editing it to UQ6f10---there's no sign bit, so they need to add to 16. Assuming you concur, I think your second block describes my preferred names. What are your thoughts, @bjarthur?

bjarthur · 2016-09-17T00:38:36Z

i'm fine with the second block. it's short, and similar to two standards (the wiki link and floating point).

re. such a large namespace. perhaps we export just the even numbers of fractional bits?

timholy · 2016-09-17T06:52:26Z

OK, updated. For now I'm exporting all types, rather than just the even ones.

Two final questions:

Should UFixed8 and UFixed16 be special-cased as U8 and U16? (For printing as well as entering the type.) We define those typealiases in Color, and an awful lot of images are of type Gray{U8} and RGB{U8}. CC @mronian.
Should we delay merging this to master until @vchuravy changes the name to Normed? And possibly synchronize with the release of the new Images, since both of these will be disruptive changes to the FPN/Colors/Images ecosystem? Another argument to delay is the possibility that the final chosen name might undergo further evolution, e.g., [RFC/WIP] unify the concepts of Fixed and UFixed #32 (comment).

bjarthur · 2016-09-18T11:40:35Z

src/FixedPointNumbers.jl

+    print(io, typechar(X))
+    f = nbitsfrac(X)
+    m = sizeof(X)*8-f-signbits(X)
+    print(io, m, 'f', f)


would it be more performant to combine these two prints into one?

Might be, I'll do that.

bjarthur · 2016-09-18T11:41:13Z

src/FixedPointNumbers.jl

    Fixed16,
    UFixed8,
+    U8,
    UFixed10,
    UFixed12,
    UFixed14,
    UFixed16,


should we deprecate UFixed16 and friends?

Yeah, I think so. Fixed16 too?

bjarthur · 2016-09-18T11:41:38Z

src/FixedPointNumbers.jl

-    # literal constructor constants
+    U16,
+    # Q and U typealiases are exported in separate source files
+# literal constructor constants
    uf8,
    uf10,
    uf12,
    uf14,
    uf16,


deprecate uf16 and friends too?

We could, but those exist for a different reason---they are "repr constructors"---that's not covered by anything else:

julia> 0xc2uf8 0.761N0f8 julia> 0.761N0f8 0.761N0f8 julia> 0xc2N0f8 ERROR: InexactError() in *(::UInt8, ::Type{FixedPointNumbers.UFixed{UInt8,8}}) at /home/tim/.julia/v0.5/FixedPointNumbers/src/FixedPointNumbers.jl:63

i don't think the casual user of julia would find this intuitive. is there nothing to be done that would make that work?

If we really don't like these, perhaps we should get rid of them.

But 0xc2N0f8 or N0f8(0xc2) can never work as I think you're intending without causing absolute chaos. See JuliaGraphics/ColorTypes.jl#49 (comment). We need to firmly keep in mind the distinction between value and representation of that value, and generally one should just ignore the representation and think about the number in terms of its value. 0xc2 = 194 is far beyond the reach of a number representable with an N0f8, so if the constructor doesn't throw an error you've got a big problem.

I think the key is to (1) reliably throw errors when the user tries something that's not allowed (we didn't do that before #48 because of arithmetic overflow), and (2) provide better error messages that actually coach users through how to think about this/fix problems, along the lines of #49 (comment).

OK, maybe I'll deprecate 0xc2uf8 in favor of reinterpret(N0f8, 0xc2). Reasonable? Forcing people to use reinterpret explicitly might help clarify everyone's thinking.

doh! i was getting confused between value and representation just as you describe.

deprecating uf8 in favor of reinterpret sounds great.

I've confused myself so many times I've lost track. But I think we're moving towards a solution that will be easier to keep straight!

bjarthur · 2016-09-18T11:49:37Z

src/ufixed.jl

 typealias UFixed8  UFixed{UInt8,8}
 typealias UFixed10 UFixed{UInt16,10}
 typealias UFixed12 UFixed{UInt16,12}
 typealias UFixed14 UFixed{UInt16,14}
 typealias UFixed16 UFixed{UInt16,16}
+typealias U16      UFixed{UInt16,16}


UFixed{UInt16,16} is already aliased as N0f16. i think that's short enough, and having another nomenclature would just add confusion.

I largely agree with this. But should we apply the same logic to U8? In the Colors/Images world, we use RGB{U8} all the time. That would become RGB{N0f8}, which is not bad, but a little more to remember. (Probably I'm just not used to it, though.)

I also think it's valid to decide to keep U8 but ditch U16. Basically, I am persuadable to any outcome on this point 😄.

yes, i'd suggest ditch U8 too. it's bad enough that signed fixed point ints are scaled differently than unsigned ones. we don't need multiple ways to specify the latter to make it even more confusing. N0f8 is already a huge improvement over the status quo.

The U8 notation is pretty intuitive for someone coming from an image processing background since most packages use the general datatypes defined in their languages as the base Image type unlike Images.jl where we have FixedPointNumbers to abstract that out and give us one less thing to worry about 😄

If we are going to remove U8 and friends, then we need to support it with good documentation in the Images.jl packages (explaining how FixedPointNumbers works, why it helps and the notation). This does seem like a big change though so if you could also have a big warning message when you deprecate U8 (something that explains how to use the Nxfy notation too) it will really help users. Apart from this, I like the new implementation and even if there is a bit of a learning curve to it, I guess its worth it in the long run 😄

bjarthur · 2016-09-19T11:20:15Z

src/FixedPointNumbers.jl

    scaledual

 reinterpret(x::FixedPoint) = x.i

+# construction using the (approximate) intended value, i.e., 0.8U⁰₈


don't forget to change this comment to match whatever format we finally decide on

timholy · 2016-10-11T19:20:08Z

OK, I've rebased this and addressed review comments. I've deprecated everything in sight, but there's a chance we'll want to rethink some of these (particularly the U8 deprecation and the 0xa0uf8 deprecations). I'll try to collect feedback from Images users.

Also note: I added a shell script in the (new) contrib folder to assist with fixing deprecations.

codecov-io · 2016-10-11T20:59:52Z

Current coverage is 82.58% (diff: 47.82%)

Merging #51 into master will decrease coverage by 5.83%

@@             master        #51   diff @@
==========================================
  Files             3          4     +1   
  Lines           164        178    +14   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            145        147     +2   
- Misses           19         31    +12   
  Partials          0          0

Powered by Codecov. Last update ccb4267...223772d

bjarthur · 2016-11-06T01:17:25Z

i believe ColorTypes.jl will have to be changed once this PR is merged to typealias U8 to N0f8

timholy · 2016-11-06T18:13:08Z

If you use imagebranch and switch to fixed-renaming, it's already done.

timholy · 2017-01-26T23:57:08Z

Heads-up @vchuravy, this will happen over the weekend. (ref #51 (comment))

This rename reflects the nature of `UFixed` it is not just a unsigned fixpoint number type (for that you can use `Fixed{T<:Unsigned}`), but rather a number type that is normalised to a specific `one`.

Rename UFixed to Normed

timholy · 2017-01-28T11:09:47Z

Bam. Let the chaos begin.

timholy force-pushed the teh/compact_printing branch from 80e602a to be32eff Compare September 16, 2016 02:22

timholy changed the title ~~RFC/WIP: compact systematic display of values~~ RFC: compact systematic display of values Sep 16, 2016

timholy mentioned this pull request Sep 16, 2016

Countdown to the new Images.jl JuliaImages/Images.jl#542

Closed

42 tasks

timholy force-pushed the teh/compact_printing branch from be32eff to 3e7e0c1 Compare September 17, 2016 07:07

bjarthur reviewed Sep 18, 2016

View reviewed changes

bjarthur reviewed Sep 19, 2016

View reviewed changes

This was referenced Sep 23, 2016

Implement promotion among UFixed, make convert errors more informative #53

Merged

Fixed8, Fixed16, etc. #57

Closed

timholy force-pushed the teh/compact_printing branch 2 times, most recently from 9a908e7 to 1aa2ba9 Compare October 11, 2016 16:10

This was referenced Oct 13, 2016

julep: Base.summary and ShowItLikeYouBuildIt JuliaLang/julia#18909

Closed

Should Union types record whether they came from a defining typealias? JuliaLang/julia#18924

Closed

Views documentation outdated? JuliaImages/ImageCore.jl#17

Closed

bjarthur mentioned this pull request Nov 22, 2016

closes https://github.com/JuliaIO/ImageMagick.jl/issues/49 JuliaIO/ImageMagick.jl#54

Merged

timholy added 4 commits January 25, 2017 13:56

Compact systematic display of values

123f990

Deprecate UFixed8 and friends in favor of N0f8 and friends

f226e62

Deprecate n*uf8 in favor of reinterpret(N0f8, n)

65689dd

Add a shell script to aid in fixing deprecations

fdf5506

timholy force-pushed the teh/compact_printing branch from 223772d to fdf5506 Compare January 25, 2017 19:57

Rename UFixed to Normed

9da3c8b

This rename reflects the nature of `UFixed` it is not just a unsigned fixpoint number type (for that you can use `Fixed{T<:Unsigned}`), but rather a number type that is normalised to a specific `one`.

timholy changed the title ~~RFC: compact systematic display of values~~ Compact systematic display of values Jan 28, 2017

timholy added 2 commits January 28, 2017 04:29

Merge pull request #63 from JuliaMath/vc/rename

af0d7a2

Rename UFixed to Normed

Remove duplicate reinterpret definition

af2a5c7

timholy merged commit 398e5dc into master Jan 28, 2017

timholy deleted the teh/compact_printing branch January 28, 2017 11:09

kimikage mentioned this pull request Sep 14, 2020

[RFC] Reserving aliases for unsigned Fixed and signed Normed #228

Open

Compact systematic display of values #51

Compact systematic display of values #51

Conversation

timholy commented Sep 15, 2016 • edited Loading

timholy commented Sep 15, 2016

timholy commented Sep 15, 2016

vchuravy commented Sep 15, 2016

timholy commented Sep 16, 2016

vchuravy commented Sep 16, 2016

timholy commented Sep 16, 2016 • edited Loading

bjarthur commented Sep 16, 2016

bjarthur commented Sep 16, 2016

bjarthur commented Sep 16, 2016

timholy commented Sep 16, 2016 • edited Loading

timholy commented Sep 16, 2016

timholy commented Sep 16, 2016

bjarthur commented Sep 16, 2016

vchuravy commented Sep 16, 2016

bjarthur commented Sep 16, 2016

timholy commented Sep 16, 2016 • edited Loading

vchuravy commented Sep 16, 2016 • edited by timholy Loading

timholy commented Sep 16, 2016

vchuravy commented Sep 16, 2016

timholy commented Sep 16, 2016

bjarthur commented Sep 17, 2016

timholy commented Sep 17, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjarthur Sep 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mronian Sep 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timholy commented Oct 11, 2016

codecov-io commented Oct 11, 2016 • edited Loading

Current coverage is 82.58% (diff: 47.82%)

bjarthur commented Nov 6, 2016

timholy commented Nov 6, 2016

timholy commented Jan 26, 2017

timholy commented Jan 28, 2017

timholy commented Sep 15, 2016 •

edited

Loading

timholy commented Sep 16, 2016 •

edited

Loading

timholy commented Sep 16, 2016 •

edited

Loading

timholy commented Sep 16, 2016 •

edited

Loading

vchuravy commented Sep 16, 2016 •

edited by timholy

Loading

timholy commented Sep 17, 2016 •

edited

Loading

bjarthur Sep 19, 2016 •

edited

Loading

mronian Sep 19, 2016 •

edited

Loading

codecov-io commented Oct 11, 2016 •

edited

Loading