Multiplication by Ufixed{UInt8, 8}(1) is no longer an identity

[cstjean]

Did FixedPoint arithmetic change recently? I'm fairly certain that this

using FixedPointNumbers: UFixed
a = UFixed{UInt8, 8}(1.0)
b = UFixed{UInt8, 8}(0.65)
@show a b a*b

> a = UFixed8(1.0)
> b = UFixed8(0.651)
> a * b = UFixed8(0.647)

was not the case until I Pkg.updated today. It's a bit of a problem for my use case...

[timholy]

Did FixedPoint arithmetic change recently?

Yes, see #27. Aside from a general cleanup, it brought FixedPointNumbers into alignment with the rest of julia 0.4 in making them closed under arithmetic operations.

But the problem you've noticed is Not Good. I've got major deadlines for the next 5 days, so I sure hope someone can help deal with this and tag a new release. CC @vchuravy?

[vchuravy]

I see what is wrong... the root of the issue is the problem I am having in #32.

So UFixed8(1) = 0xFF and widemul(0xFF, 0xFF) = 0x000000000000fe01, shift that back 8 bits and you get 0xFE01 even with the additional rounding that we do in the multiplication you do not get back to one. I will look into this tomorrow morning and in the worst case restore the old definition (, but still making it closed).

[cstjean]

Thank you. I think #27 was a great step forward, I hope the issue can be fixed!

[vchuravy]

Yeah the issue is more or less of [me] wanting to define UFixed numbers as traditional FixedPoint numbers that are just unsigned, while they are not :) and me realizing that after the PR went through.

[timholy]

If they need to be split out with different naming, I'm fine with that. I think we went through some discussion along these lines when they were first added, but that can be revisted.