convert(Type{Float16}, Float32) bug

[StefanKarpinski]

This doesn't work:

julia> convert(Float16,0) |> typeof
ERROR: no method convert(Type{Float16}, Float32)
 in convert at float.jl:13

Yet if you look at the definition of this conversion, it is this:

convert(::Type{Float16}, x::Union(Signed,Unsigned)) = convert(Float16, convert(Float32,x))

and doing this explicitly works just fine:

julia> convert(Float16,convert(Float32,0)) |> typeof
Float16

[StefanKarpinski]

This bug is what's holding up the float ranges patch: #5636.

[kmsquire]

That code works fine on 64-bit linux (with both a brand new build, and a build from Feb 12th):

julia> convert(Float16,0)
float16(0.0)

julia> convert(Float16,0) |> typeof
Float16

Did you do anything else before running that?

[quinnj]

Works on latest Windows binary and up-to-date native build too.

[ivarne]

I do not get any errors on OSX either.

[mbauman]

The current master is fine for me, too. It only happens when I checkout and build from your upstream/sk/floatrange branch.

[StefanKarpinski]

Yes, sorry. To be clear, this only happens on that branch, which is mystifying because there's nothing I can see in that change that really ought to have this kind of effect. The only thing I can think of is that it somehow changes range behavior in a way breaks something in type inference.

[StefanKarpinski]

I've now refactored the pull request so that everything works until the last commit – which only adds the colon syntax to the already-existing FloatRange type. So that narrows down the problem a bit. Still have no idea what's going on though.

[kmsquire]

Moving the definition of colon after include("multimedia.jl") (but not before) in sysimg.jl seems to allow things to work, although it did reveal a real bug:

    JULIA test/all
    From worker 2:       * linalg1
    From worker 3:       * linalg2
    From worker 3:       * core
    From worker 3:       * keywordargs
    From worker 3:       * numbers
    From worker 2:       * strings
    From worker 2:       * collections
    From worker 2:       * hashing
    From worker 2:       * remote
    From worker 2:       * iobuffer
    From worker 2:       * arrayops
    From worker 3:       * blas
    From worker 3:       * fft
    From worker 3:       * dsp
    From worker 3:       * sparse
    From worker 2:       * bitarray
    From worker 3:       * random
exception on 3: ERROR: no method round(Float16)
 in frange at range.jl:136
 in colon at /home/kmsquire/Source/julia/base/sysimg.jl:122
 in anonymous at no file:14
 in runtests at /home/kmsquire/Source/julia/test/testdefs.jl:5
 in anonymous at multi.jl:822
 in run_work_thunk at multi.jl:590
 in anonymous at task.jl:822

[StefanKarpinski]

Ah yes. This is why I had removed Float16 from test/random.jl – it's a storage type, not really an arithmetic type, so I felt that adding more and more features to it like this was not what we wanted.

[StefanKarpinski]

@kmsquire, I can't reproduce that. Can you post a patch for what you did?

[kmsquire]

Moving the definition of colon after include("multimedia.jl") (but not before) in sysimg.jl seems to allow things to work...

Sorry, I meant "multidimensional.jl". (I knew it was multi-something... ;-) )

Here's the diff:

diff --git a/base/range.jl b/base/range.jl
index 48d60ab..68132ad 100644
--- a/base/range.jl
+++ b/base/range.jl
@@ -153,12 +153,6 @@ function frange{T<:FloatingPoint}(start::T, step::T, stop::T)
     start, step, one(step), floor(r)+1
 end

-colon{T<:FloatingPoint}(start::T, step::T, stop::T) =
-          step == 0              ? error("step cannot be zero in colon syntax") :
-         start == stop           ? FloatRange{T}(start,step,1,1) :
-    (0 < step) != (start < stop) ? FloatRange{T}(start,step,1,0) :
-                                   FloatRange{T}(frange(start,step,stop)...)
-
 similar(r::Ranges, T::Type, dims::Dims) = Array(T, dims)

 length(r::Ranges) = integer(r.len)
diff --git a/base/sysimg.jl b/base/sysimg.jl
index 0f50460..d979602 100644
--- a/base/sysimg.jl
+++ b/base/sysimg.jl
@@ -119,6 +119,13 @@ include("cartesian.jl")
 using .Cartesian
 include("multidimensional.jl")

+colon{T<:FloatingPoint}(start::T, step::T, stop::T) =
+          step == 0              ? error("step cannot be zero in colon syntax") :
+         start == stop           ? FloatRange{T}(start,step,1,1) :
+    (0 < step) != (start < stop) ? FloatRange{T}(start,step,1,0) :
+                                   FloatRange{T}(frange(start,step,stop)...)
+
+
 # core math functions
 include("floatfuncs.jl")
 include("math.jl")

[kmsquire]

Update: placing the new definition of colon after the definition of checksize in multidimensional.jl (which is the first definition in the file) allows things to work.

@timholy, any ideas on why the definition of colon here would cause problems with this? On the surface, the error seems to have nothing to do with these definitions (see the description above).

[timholy]

Oh, it's obvious. You forgot to frobulate the nintingle.

[timholy]

Also note that if you just define that conversion function again,

julia> import Base.convert

julia> convert(::Type{Float16}, x::Union(Signed,Unsigned)) = convert(Float16, convert(Float32,x))
Warning: Method definition convert(Type{Float16},Union(Signed,Unsigned)) in module Base at float.jl:13 overwritten in module Main at none:1.
convert (generic function with 377 methods)

then it starts working again.

Stefan's theory that defining colon somehow causes a type inference problem seems good, although I'd be surprised if anything in inference were using floating-point ranges. Alternatively, perhaps there's a "deep" Julia bug that this is somehow uncovering?

[kmsquire]

Thanks for looking. Neither frobulating nor defrobulating seem to help (but what's a nintingle? ;-)

[timholy]

I think it's defined on the notsafeforwork branch 😉.

[JeffBezanson]

This is #265-related. During bootstrap some code (in cartesian.jl) makes a range, which causes all the nextfloat methods to be analyzed, which in turn looks at sign(Float16), which uses conversion from Int to Float16, but it is not defined yet since float16.jl hasn't been loaded yet. So yes this is a deep bug, but it also doesn't make much sense to include float16.jl so long after float.jl, which uses many functions from it.

[StefanKarpinski]

Yikes. Thanks for finding all of that.