Skip to content
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.

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

Type inference failure on round broadcast #329

Closed
schmrlng opened this issue Nov 1, 2017 · 5 comments
Closed

Type inference failure on round broadcast #329

schmrlng opened this issue Nov 1, 2017 · 5 comments

Comments

@schmrlng
Copy link
Contributor

schmrlng commented Nov 1, 2017

On StaticArrays.jl master with Julia 0.6.1:

  | | |_| | | | (_| |  |  Version 0.6.1 (2017-10-24 22:15 UTC)
 _/ |\__'_|_|_|\__'_|  |  
|__/                   |  x86_64-linux-gnu

julia> using StaticArrays

julia> Core.Inference.return_type(x -> round.(Int, x), (Vector{Float64},))
Array{Int64,1}

julia> Core.Inference.return_type(x -> round.(Int, x), (SVector{4,Float64},))
SVector{4,Any}

julia> f(v) = round.(Int, v)
f (generic function with 1 method)

julia> f(SVector(1.,2.,3.))
3-element SVector{3,Any}:
 1
 2
 3

Yet somehow this works fine:

julia> round.(Int, SVector(1.,2.,3.))
3-element SVector{3,Int64}:
 1
 2
 3

Maybe an issue with base Julia, not with StaticArrays?

@andyferris
Copy link
Member

Hmm... an interesting one.

Here's another that works:

julia> g(v) = broadcast(x -> round(Int,x), v)
g (generic function with 1 method)

julia> g(v)
3-element SVector{3,Int64}:
 1
 2
 3

The two likely causes: (1) our broadcast implementation is not pure enough, or (2) there may be some bug in the compiler.

@c42f
Copy link
Member

c42f commented Nov 1, 2017

I think it's not exactly a bug in julia... more an unimplemented feature, perhaps. The fact that we return a StaticArray of Any rather than just determining the type dynamically arguably is a bug.

I've got a partial fix for this, which as a side effect also removes all explicit uses of Base.Inference.return_type, will make a PR soon.

c42f added a commit that referenced this issue Nov 1, 2017
With this approach based on the eltype of tuple of computed elements, we
get the correct dynamic result independently of whether inference can
infer the return type.

This fixes #198, and partially addresses #329 but breaks a previous test
case.
@c42f
Copy link
Member

c42f commented Nov 1, 2017

Ok, I actually have a full workaround for this (I think) in #330, pending tests passing everywhere.

Perhaps the cause is something to do with the way inference constant propagates types, with the Int being passed to round via several layers of function calls (does it somehow degenerate to being inferred as DataType)? Speculation really, I haven't tried to track this down in inference.

@mschauer
Copy link
Collaborator

mschauer commented Nov 1, 2017

If have seen the function Base.Inference.return_type giving Any without inference actually failing before.

@c42f
Copy link
Member

c42f commented Nov 1, 2017

That's pretty weird. In this case though, even avoiding using return_type explicitly didn't help and I had to insert a hack to override broadcast explicitly. The required form seemed oddly precise, involving capturing the type into a closure before it went into the layers of broadcast machinery.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants