Empower Fixed and demotes UFixed.

[vchuravy]

Supersedes #32.

Instead of doing the big overhaul I decided to do it piecewise. (If anything is controversial we can also use 0a8e222 independently to close #34).

I adopted most changes from #32 and the next step would be to rename UFixed and deprecate its usage to free UFixed to be used as unsigned fixed-point and to rework the UFixed implementation to properly reflect its normalized nature.

[vchuravy]

Next step is to rename UFixed and to deprecate it as a name. How about UNorm?

[timholy]

This is interesting, but I worry it might give people the wrong idea that there's some kind of tuple or array container inside. So I'm on the fence about it. Care to provide a little explanation?

[vchuravy]

When going through the codebase I found that reinterpret(x) and x.i where both used, thus defeating the point of using reinterpret(x), also for me personally reinterpret makes it harder to read or quickly parse the code. So my main issue with it is that it is to long and it gets in the way. I was thinking about using get(x), but it is semantically as problematic as getindex.

[timholy]

The problem, however, with your solution is that it conflicts with

julia> 5[]
5

julia> 3.2[]
3.2

So anyone who writes code like

function mysum(X)
    s = zero(first(X))
    for x in X
        s += x
    end
    s
end

will get a very surprising answer from mysum(0xffuf8).

x.i might be used inside FixedPointNumbers, but I've never seen such usage outside. If this is intended only for internal use, rather than overload an exported function from Base you could define raw (or something) as an internal function.

[vchuravy]

I see your point, I will change it to raw.

On Tue, 3 Nov 2015 18:33 Tim Holy [email protected] wrote:

In src/FixedPointNumbers.jl
#35 (comment)
:

@@ -40,54 +41,87 @@ export
# Functions
scaledual

-reinterpret(x::FixedPoint) = x.i
+getindex(x::AbstractFixedPoint) = x.i

The problem, however, with your solution is that it conflicts with

julia> 5[]5

julia> 3.2[]3.2

So anyone who writes code like

function mysum(X)
s = zero(first(X))
for x in X
s += x
end
s

will get a very surprising answer from mysum(0xffuf8).

x.i might be used inside FixedPointNumbers, but I've never seen such
usage outside. If this is intended only for internal use, rather than
overload an exported function from Base you could define raw (or
something) as an internal function.

—
Reply to this email directly or view it on GitHub
https://github.com/JeffBezanson/FixedPointNumbers.jl/pull/35/files#r43728468
.

[timholy]

I'm fine with the name UNorm, but as I read a little more widely I am not sure the change is necessary. Since not a lot of people watch this repo, I'm CCing some of the main contributors in the Colors and Images worlds: @rsrock, @kmsquire, @lucasb-eyer, @tkelman, @SimonDanisch, @dcjones, @jiahao, @glenn-sweeney, @m-lohmann, @stevengj. Also @StefanKarpinski, since he was the one who made the initial suggestion that launched the UFixed types, and @simonbyrne, since he is always a good person to consult with on anything related to numbers.

Background: what we now call UFixed numbers are a little bit confusing in their classification. Some sources essentially define a fixed-point number as using a fixed number of bits to represent the fractional part---in this package that's true for the Fixed type but not the UFixed type, since 0xff maps to 1.0 but all other UFixed8 values map to something fractional. This motivates changing the name to something that doesn't have "fixed" in it. However, other sources don't define them in terms of a binary representation, in which case the UFixed name is actually fine---these really are genuine fixed-point numbers, with a scaling factor (for UFixed8) 0xff. However, the presence of a type Fixed{T,f}, where f means "number of bits," has the consequence that the U in UFixed is a bit misleading as far as conveying the most important distinction between these types. #32 was an attempt to unify Fixed and UFixed that failed because of the misleading nature of these names.

This analysis does suggest a potential way to unify these numbers successfully. Rather than Fixed{T,f} (with e.g., f=8), perhaps it should be parametrized as Fixed{T,One}. Our current UFixed8 might then be Fixed{UInt8,0xff}. I suspect, however, that this parametrization would eliminate the chances for certain optimizations (at least, not unless we switch to using @generated functions for everything, and I know how that suggestion will be received 😄). So perhaps we'd need a FixedBinary{T,f} type as well as the more general type.

I don't want unnecessary churn for users (especially since we went through a whole bunch of deprecation warnings very recently!), but in the long run having appropriate names matters. So it's an important discussion. I see at least three options:

• leave things as they are
• change UFixed to UNorm
• change Fixed{T,f} to FixedBinary{T,f}, after a deprecation period introduce a more general Fixed{T,One} and base the UFixed numbers on the new Fixed. Or we could get there faster if we just switch a new name like FixedPointNumber{T,One}.

I'll add one more comment: one factor worth bearing in mind is that there are presumably very few users (currently) of the general Fixed type, whereas the UFixed types (because of their usage in Colors and Images) are widely used. So whatever we do, we don't want to come up with a naming or parametrization that makes life more unpleasant for >95% of the practical uses of these numbers.

Anyone care to comment?

[timholy]

(I'd also like to applaud @vchuravy for his continuing efforts here, as well as his interest/willingness to see things done right.)

[vchuravy]

@timholy I am happy to be of help, especially since the whole thing started with me not understanding the fine differences between UFixed and what I thought it should do.

I will rework the PR over the weekend to address your comments.

[kmsquire]

Perhaps the reason for the lack of responses here is that it's hard to see the best solution. UFixed certainly is useful for color values (since 0xFF = 1.0 represents saturation), but I agree that renaming it would probably be a good idea.

UNorm is concise and sounds okay to me. It might generalize to UScaled if the range of binary values needs to represent a closed interval. But I don't know how useful this would be.

Cheers!

[StefanKarpinski]

What we need here is a good term for a value from the unit interval. Nothing's coming to mind...

[bjarthur]

@timholy what did you mean by this comment:

(at least, not unless we switch to using @generated functions for everything, and I know how that suggestion will be received 😄)

i ask, because i just spent the day refactoring UFixed to use generated functions, so that UFixed{UInt32,30}, for example, was supported, and now i wonder whether i should even bother to submit a PR.

[StefanKarpinski]

Oh uh. Using generated functions everywhere is not the direction we need to head as a language, but if you make a PR maybe we can figure out another way to do this. If some kind of generated type seems to be necessary for certain classes of problem, then perhaps we can add support for that to the language. Using generated functions to define types is not ideal (and not supported in 0.5).

[bjarthur]

PR with just two generated functions submitted: #44. neither has side effects nor references global variables, so i believe i'm using them exactly as intended, as described here.

[vchuravy]

Closing for now till I have time to revisit this.