Skip to content

[RFC] Commonize further code between Fixed and Normed #168

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 16, 2020
Merged

[RFC] Commonize further code between Fixed and Normed #168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f79606a
Commonize `Bool` and `Integer` conversions
kimikage Dec 25, 2019
983c3f6
Commonize `Rational` conversions
kimikage Dec 25, 2019
497f4ba
Commonize constructor-style conversions
kimikage Dec 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion src/FixedPointNumbers.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,28 @@ nbitsfrac(::Type{X}) where {T, f, X <: FixedPoint{T,f}} = f
rawtype(::Type{X}) where {T, X <: FixedPoint{T}} = T

# construction using the (approximate) intended value, i.e., N0f8
*(x::Real, ::Type{X}) where {X<:FixedPoint} = X(x)
*(x::Real, ::Type{X}) where {X <: FixedPoint} = _convert(X, x)

# constructor-style conversions
(::Type{X})(x::Real) where {X <: FixedPoint} = _convert(X, x)

function (::Type{<:FixedPoint})(x::AbstractChar)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we still need this. Do you fail the ambiguity check if you delete it? (I ask because the supertype of AbstractChar is now Any but IIRC it used to be Integer.)

Copy link
Collaborator Author

@kimikage kimikage Jan 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the rawtype T should not be an AbstractChar type. However, whether to allow conversion from a Char is up to the implementation, regardless of what its supertype is. I left this fool-proof just because I had no reason to remove it. So, Let's get rid of it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI:
JuliaLang/julia#8816
JuliaLang/julia#26286
It seems that AbstractChar was added long after the supertype of Char had been changed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is that this was added purely as a means to avoid an ambiguity. In the old days, ambiguities printed as warnings when you loaded the package, which was really annoying.

JuliaLang/julia#6190
JuliaLang/julia#16125

Copy link
Collaborator Author

@kimikage kimikage Jan 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trigger is certainly the ambiguities. (cf. PR #105)
What I want to say in the comment above is that this may be intentionally left as a fool-proof.
I thought I would get a MethodError, but the conversion is done without errors.

julia> Normed{UInt32,16}('a')
97.0N16f16

throw(ArgumentError("FixedPoint (Fixed or Normed) cannot be constructed from a Char"))
end
(::Type{X})(x::Complex) where {X <: FixedPoint} = X(convert(real(typeof(x)), x))
function (::Type{X})(x::Base.TwicePrecision) where {X <: FixedPoint}
floattype(X) === BigFloat ? X(big(x)) : X(convert(floattype(X), x))
end

# conversions
function Base.Bool(x::FixedPoint)
x == zero(x) ? false : x == oneunit(x) ? true : throw(InexactError(:Bool, Bool, x))
end
function (::Type{Ti})(x::FixedPoint) where {Ti <: Integer}
isinteger(x) || throw(InexactError(:Integer, typeof(x), x))
floor(Ti, x)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to combine these (compute the output first and then check for equality)? Or does that miss some corner cases?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FixedPointNumbers doesn't know where or what errors floor(Ti, x) throws. So, it is better to check first for a clear error message.
I don't want to optimize the methods which can throw errors, in principle.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, when isinteger uses floor, it can be optimized simply. It was obvious to me, so I forgot to write a TODO comment. 😄

end
Base.Rational{Ti}(x::FixedPoint) where {Ti <: Integer} = Rational{Ti}(Rational(x))

"""
isapprox(x::FixedPoint, y::FixedPoint; rtol=0, atol=max(eps(x), eps(y)))
Expand Down
48 changes: 21 additions & 27 deletions src/fixed.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,6 @@ struct Fixed{T <: Signed, f} <: FixedPoint{T, f}
end
end

Fixed{T, f}(x::AbstractChar) where {T,f} = throw(ArgumentError("Fixed cannot be constructed from a Char"))
Fixed{T, f}(x::Complex) where {T,f} = Fixed{T, f}(convert(real(typeof(x)), x))
Fixed{T, f}(x::Base.TwicePrecision) where {T,f} = Fixed{T, f}(convert(Float64, x))
Fixed{T,f}(x::Integer) where {T,f} = Fixed{T,f}(round(T, convert(widen1(T),x)<<f),0)
Fixed{T,f}(x::AbstractFloat) where {T,f} = Fixed{T,f}(round(T, trunc(widen1(T),x)<<f + rem(x,1)*(one(widen1(T))<<f)),0)
Fixed{T,f}(x::Rational) where {T,f} = Fixed{T,f}(x.num)/Fixed{T,f}(x.den)

typechar(::Type{X}) where {X <: Fixed} = 'Q'
signbits(::Type{X}) where {X <: Fixed} = 1

Expand All @@ -53,6 +46,25 @@ end
intmask(::Fixed{T,f}) where {T, f} = -oneunit(T) << f # Signed
fracmask(x::Fixed{T,f}) where {T, f} = ~intmask(x) # Signed

# constructor-style conversions
function _convert(::Type{F}, x::Fixed{T2,f2}) where {T, T2, f, f2, F <: Fixed{T,f}}
y = round(((1<<f)/(1<<f2))*reinterpret(x)) # FIXME: avoid overflow
(typemin(T) <= y) & (y <= typemax(T)) || throw_converterror(F, x)
reinterpret(F, _unsafe_trunc(T, y))
end

function _convert(::Type{F}, x::Integer) where {T, f, F <: Fixed{T,f}}
reinterpret(F, round(T, convert(widen1(T),x)<<f)) # TODO: optimization and input range checking
end

function _convert(::Type{F}, x::AbstractFloat) where {T, f, F <: Fixed{T,f}}
reinterpret(F, round(T, trunc(widen1(T),x)<<f + rem(x,1)*(one(widen1(T))<<f))) # TODO: optimization and input range checking
end

function _convert(::Type{F}, x::Rational) where {T, f, F <: Fixed{T,f}}
F(x.num)/F(x.den) # TODO: optimization and input range checking
end

# unchecked arithmetic

# with truncation:
Expand All @@ -63,15 +75,6 @@ fracmask(x::Fixed{T,f}) where {T, f} = ~intmask(x) # Signed
/(x::Fixed{T,f}, y::Fixed{T,f}) where {T,f} = Fixed{T,f}(div(convert(widen(T), x.i) << f, y.i), 0)


# # conversions and promotions
function Fixed{T,f}(x::Fixed{T2,f2}) where {T <: Integer,T2 <: Integer,f,f2}
# reinterpret(Fixed{T,f},T(reinterpret(x)<<(f-f2)))
U = Fixed{T,f}
y = round(((1<<f)/(1<<f2))*reinterpret(x))
(typemin(T) <= y) & (y <= typemax(T)) || throw_converterror(U, x)
reinterpret(U, _unsafe_trunc(T, y))
end

rem(x::Integer, ::Type{Fixed{T,f}}) where {T,f} = Fixed{T,f}(rem(x,T)<<f,0)
rem(x::Real, ::Type{Fixed{T,f}}) where {T,f} = Fixed{T,f}(rem(Integer(trunc(x)),T)<<f + rem(Integer(round(rem(x,1)*(one(widen1(T))<<f))),T),0)

Expand All @@ -81,19 +84,10 @@ Base.BigFloat(x::Fixed{T,f}) where {T,f} =
(::Type{TF})(x::Fixed{T,f}) where {TF <: AbstractFloat,T,f} =
TF(x.i>>f) + TF(x.i&(one(widen1(T))<<f - 1))/TF(one(widen1(T))<<f)

Base.Bool(x::Fixed{T,f}) where {T,f} = x.i!=0
function Base.Integer(x::Fixed{T,f}) where {T,f}
isinteger(x) || throw(InexactError())
Integer(x.i>>f)
end
function (::Type{TI})(x::Fixed{T,f}) where {TI <: Integer,T,f}
isinteger(x) || throw(InexactError())
TI(x.i>>f)
function Base.Rational(x::Fixed{T,f}) where {T, f}
f < bitwidth(T)-1 ? x.i//rawone(x) : x.i//(one(widen1(T))<<f)
end

(::Type{TR})(x::Fixed{T,f}) where {TR <: Rational,T,f} =
TR(x.i>>f + (x.i&(1<<f-1))//(one(widen1(T))<<f))

function trunc(x::Fixed{T,f}) where {T, f}
f == 0 && return x
f == bitwidth(T) && return zero(x) # TODO: remove this line
Expand Down
51 changes: 23 additions & 28 deletions src/normed.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ struct Normed{T <: Unsigned, f} <: FixedPoint{T, f}
end
end

Normed{T, f}(x::AbstractChar) where {T,f} = throw(ArgumentError("Normed cannot be constructed from a Char"))
Normed{T, f}(x::Complex) where {T,f} = Normed{T, f}(convert(real(typeof(x)), x))
Normed{T, f}(x::Base.TwicePrecision) where {T,f} = Normed{T, f}(convert(Float64, x))
Normed{T1,f}(x::Normed{T2,f}) where {T1 <: Unsigned,T2 <: Unsigned,f} = Normed{T1,f}(convert(T1, x.i), 0)

typechar(::Type{X}) where {X <: Normed} = 'N'
signbits(::Type{X}) where {X <: Normed} = 0

Expand All @@ -42,34 +37,38 @@ function rawone(::Type{Normed{T,f}}) where {T <: Unsigned, f}
typemax(T) >> (bitwidth(T) - f)
end

# Conversions
function Normed{T,f}(x::Normed{T2}) where {T <: Unsigned,T2 <: Unsigned,f}
U = Normed{T,f}
y = round((rawone(U)/rawone(x))*reinterpret(x))
(0 <= y) & (y <= typemax(T)) || throw_converterror(U, x)
reinterpret(U, _unsafe_trunc(T, y))
end
N0f16(x::N0f8) = reinterpret(N0f16, convert(UInt16, 0x0101*reinterpret(x)))
# constructor-style conversions
function _convert(::Type{N}, x::Normed{T2,f}) where {T, T2, f, N <: Normed{T,f}}
reinterpret(N, convert(T, x.i)) # TODO: input range checking
end

(::Type{U})(x::Real) where {U <: Normed} = _convert(U, x)
function _convert(::Type{N}, x::Normed{T2,f2}) where {T, T2, f, f2, N <: Normed{T,f}}
y = round((rawone(N)/rawone(x))*reinterpret(x))
(0 <= y) & (y <= typemax(T)) || throw_converterror(N, x)
reinterpret(N, _unsafe_trunc(T, y))
end

function _convert(::Type{N}, x::Normed{UInt8,8}) where {N <: Normed{UInt16,16}} # TODO: generalization
reinterpret(N0f16, convert(UInt16, 0x0101*reinterpret(x)))
end

function _convert(::Type{U}, x) where {T, f, U <: Normed{T,f}}
function _convert(::Type{N}, x::Real) where {T, f, N <: Normed{T,f}}
if T == UInt128 # for UInt128, we can't widen
# the upper limit is not exact
(0 <= x) & (x <= (typemax(T)/rawone(U))) || throw_converterror(U, x)
y = round(rawone(U)*x)
(0 <= x) & (x <= (typemax(T)/rawone(N))) || throw_converterror(N, x)
y = round(rawone(N)*x)
else
y = round(widen1(rawone(U))*x)
(0 <= y) & (y <= typemax(T)) || throw_converterror(U, x)
y = round(widen1(rawone(N))*x)
(0 <= y) & (y <= typemax(T)) || throw_converterror(N, x)
end
reinterpret(U, _unsafe_trunc(T, y))
reinterpret(N, _unsafe_trunc(T, y))
end
# Prevent overflow (https://discourse.julialang.org/t/saving-greater-than-8-bit-images/6057)
function _convert(::Type{U}, x::Float16) where {T, f, U <: Normed{T,f}}
if Float16(typemax(T)/rawone(U)) > Float32(typemax(T)/rawone(U))
x == Float16(typemax(T)/rawone(U)) && return typemax(U)
function _convert(::Type{N}, x::Float16) where {T, f, N <: Normed{T,f}}
if Float16(typemax(T)/rawone(N)) > Float32(typemax(T)/rawone(N))
x == Float16(typemax(T)/rawone(N)) && return typemax(N)
end
return _convert(U, Float32(x))
return _convert(N, Float32(x))
end
function _convert(::Type{N}, x::Tf) where {T, f, N <: Normed{T,f}, Tf <: Union{Float32, Float64}}
if T === UInt128 && f == 53
Expand Down Expand Up @@ -239,10 +238,6 @@ end

Base.BigFloat(x::Normed) = reinterpret(x)*(1/BigFloat(rawone(x)))

Base.Bool(x::Normed) = x == zero(x) ? false : true
Base.Integer(x::Normed) = convert(Integer, x*1.0)
(::Type{T})(x::Normed) where {T <: Integer} = convert(T, x*(1/oneunit(T)))
Base.Rational{Ti}(x::Normed) where {Ti <: Integer} = convert(Ti, reinterpret(x))//convert(Ti, rawone(x))
Base.Rational(x::Normed) = reinterpret(x)//rawone(x)

abs(x::Normed) = x
Expand Down
27 changes: 27 additions & 0 deletions test/fixed.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,16 @@ end
@test_throws InexactError convert(Fixed{Int8, 7}, 1)
@test_throws InexactError convert(Fixed{Int8, 7}, 2)
@test_throws InexactError convert(Fixed{Int8, 7}, 128)

@test convert(Q2f5, -1//2) === -0.5Q2f5
@test_broken convert(Q1f6, Rational{Int8}(-3//4)) === -0.75Q1f6
@test_broken convert(Q0f7, Rational{Int16}(-3//4)) === -0.75Q0f7
@test_broken convert(Q0f7, Rational{UInt8}(3//4)) === 0.75Q0f7

@test convert(Q0f7, Base.TwicePrecision(0.5)) === 0.5Q0f7
@test_throws InexactError convert(Q7f8, Base.TwicePrecision(0x80, 0x01))
tp = Base.TwicePrecision(0xFFFFFFFFp-32, 0xFFFFFFFEp-64)
@test convert(Q0f63, tp) === reinterpret(Q0f63, typemax(Int64))
end

@testset "test_fixed" begin
Expand Down Expand Up @@ -231,9 +241,26 @@ end
end
end

@testset "bool conversions" begin
@test convert(Bool, 0.0Q1f6) === false
@test convert(Bool, 1.0Q1f6) === true
@test_throws InexactError convert(Bool, 0.5Q1f6)
@test_throws InexactError convert(Bool, -1Q1f6)
@test_broken convert(Bool, Fixed{Int8,8}(0.2)) # TODO: remove this
end

@testset "Integer conversions" begin
@test convert(Int, Q1f6(1)) === 1
@test convert(Integer, Q1f6(1)) === Int8(1)
@test convert(UInt, 1Q1f6) === UInt(1)
@test_throws InexactError convert(Integer, 0.5Q1f6)
@test_throws InexactError convert(Int8, 256Q9f6)
end

@testset "rational conversions" begin
@test convert(Rational, -0.75Q1f6) === Rational{Int8}(-3//4)
@test convert(Rational, -0.75Q0f7) === Rational{Int16}(-3//4)
@test convert(Rational{Int}, -0.75Q0f7) === Rational(-3//4)
end

@testset "Floating-point conversions" begin
Expand Down
56 changes: 35 additions & 21 deletions test/normed.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,19 @@ end

@test convert(N0f8, 1.1f0/typemax(UInt8)) == eps(N0f8)

@test_broken convert(N0f8, 1//255) === eps(N0f8)
@test_broken convert(N0f8, Rational{Int8}(3//5)) === N0f8(3/5)
@test_broken convert(N0f8, Rational{UInt8}(3//5)) === N0f8(3/5)

@test convert(N0f8, Base.TwicePrecision(1.0)) === 1N0f8

@test convert(Float64, eps(N0f8)) == 1/typemax(UInt8)
@test convert(Float32, eps(N0f8)) == 1.0f0/typemax(UInt8)
@test convert(BigFloat, eps(N0f8)) == BigFloat(1)/typemax(UInt8)
for T in (FixedPointNumbers.UF..., UF2...)
@test convert(Bool, zero(T)) == false
@test convert(Bool, one(T)) == true
@test convert(Bool, convert(T, 0.2)) == true
@test_throws InexactError convert(Bool, convert(T, 0.2))
@test convert(Int, one(T)) == 1
@test convert(Integer, one(T)) == 1
@test convert(Rational, one(T)) == 1
Expand All @@ -109,30 +115,38 @@ end
@test convert(Normed{UInt16,7}, Normed{UInt8,7}(0.504)) === Normed{UInt16,7}(0.504)
end

@testset "integer conversions" begin
@test convert(UInt, 1N1f7) === UInt(1)
@test convert(Integer, 1N1f7) === 0x01
@test convert(Int, 1N1f7) === 1
@test_throws InexactError convert(Integer, 0.5N1f7)
@test_throws InexactError convert(Int8, 256N8f8)
end

@testset "conversion from float" begin
# issue 102
for T in (UInt8, UInt16, UInt32, UInt64, UInt128)
for Tf in (Float16, Float32, Float64)
@testset "Normed{$T,$f}(::$Tf)" for f = 1:bitwidth(T)
U = Normed{T,f}
r = FixedPointNumbers.rawone(U)
N = Normed{T,f}
r = FixedPointNumbers.rawone(N)

@test reinterpret(U(zero(Tf))) == 0x0
@test reinterpret(N(zero(Tf))) == 0x0

input_typemax = Tf(typemax(U))
input_typemax = Tf(typemax(N))
if isinf(input_typemax)
@test reinterpret(U(floatmax(Tf))) >= round(T, floatmax(Tf))
@test reinterpret(N(floatmax(Tf))) >= round(T, floatmax(Tf))
else
@test reinterpret(U(input_typemax)) >= (typemax(T)>>1) # overflow check
@test reinterpret(N(input_typemax)) >= (typemax(T)>>1) # overflow check
end

input_upper = Tf(BigFloat(typemax(T)) / r, RoundDown)
isinf(input_upper) && continue # for Julia v0.7
@test reinterpret(U(input_upper)) == T(min(round(BigFloat(input_upper) * r), typemax(T)))
@test reinterpret(N(input_upper)) == T(min(round(BigFloat(input_upper) * r), typemax(T)))

input_exp2 = Tf(exp2(bitwidth(T) - f))
isinf(input_exp2) && continue
@test reinterpret(U(input_exp2)) == T(input_exp2) * r
@test reinterpret(N(input_exp2)) == T(input_exp2) * r
end
end
end
Expand All @@ -149,27 +163,27 @@ end
end

for Tf in (Float16, Float32, Float64)
@testset "$Tf(::Normed{$Ti})" for Ti in (UInt8, UInt16)
@testset "$Tf(::Normed{$Ti,$f})" for f = 1:bitwidth(Ti)
T = Normed{Ti,f}
@testset "$Tf(::Normed{$T})" for T in (UInt8, UInt16)
@testset "$Tf(::Normed{$T,$f})" for f = 1:bitwidth(T)
N = Normed{T,f}
float_err = 0.0
for i = typemin(Ti):typemax(Ti)
f_expected = Tf(i / BigFloat(FixedPointNumbers.rawone(T)))
for i = typemin(T):typemax(T)
f_expected = Tf(i / BigFloat(FixedPointNumbers.rawone(N)))
isinf(f_expected) && break # for Float16(::Normed{UInt16,1})
f_actual = Tf(reinterpret(T, i))
f_actual = Tf(reinterpret(N, i))
float_err += abs(f_actual - f_expected)
end
@test float_err == 0.0
end
end
@testset "$Tf(::Normed{$Ti})" for Ti in (UInt32, UInt64, UInt128)
@testset "$Tf(::Normed{$Ti,$f})" for f = 1:bitwidth(Ti)
T = Normed{Ti,f}
@testset "$Tf(::Normed{$T})" for T in (UInt32, UInt64, UInt128)
@testset "$Tf(::Normed{$T,$f})" for f = 1:bitwidth(T)
N = Normed{T,f}
error_count = 0
for i in vcat(Ti(0x00):Ti(0xFF), (typemax(Ti)-0xFF):typemax(Ti))
f_expected = Tf(i / BigFloat(FixedPointNumbers.rawone(T)))
for i in vcat(T(0x00):T(0xFF), (typemax(T)-0xFF):typemax(T))
f_expected = Tf(i / BigFloat(FixedPointNumbers.rawone(N)))
isinf(f_expected) && break # for Float16() and Float32()
f_actual = Tf(reinterpret(T, i))
f_actual = Tf(reinterpret(N, i))
f_actual == f_expected && continue
f_actual == prevfloat(f_expected) && continue
f_actual == nextfloat(f_expected) && continue
Expand Down