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

Inference regression for empty array #20037

Closed
nalimilan opened this issue Jan 14, 2017 · 10 comments
Closed

Inference regression for empty array #20037

nalimilan opened this issue Jan 14, 2017 · 10 comments
Labels
needs decision A decision on this change is needed regression Regression in behavior compared to a previous version

Comments

@nalimilan
Copy link
Member

DataFrames formula code hits an inference regression with an empty array. This is problematic since ! is now only defined for AbstractArray{Bool} (which might be an issue on its own).

function f()
    x = [1]
    x = x[!(x .== 1)]
    x .== 0
end

function g()
    x = Any[1]
    x = x[!(x .== 1)]
    x .== 0
end

function h()
    x = Any[1]
    x .== 0
end

On Julia 0.5.0:

julia> f()
0-element BitArray{1}

julia> g()
0-element BitArray{1}

julia> h()
1-element BitArray{1}:
 false

On master:

julia> f()
0-element BitArray{1}

julia> g()
0-element Array{Any,1}

julia> h()
1-element BitArray{1}:
 false
@ararslan ararslan added compiler:inference Type inference regression Regression in behavior compared to a previous version labels Jan 14, 2017
@Sacha0
Copy link
Member

Sacha0 commented Jan 14, 2017

(Tangentially, perhaps we should deprecate ! vectorized over AbstractArray{Bool}? Also, !.[true] yields ERROR: syntax: invalid identifier name "." (#20039). Best!)

@pabloferz
Copy link
Contributor

pabloferz commented Jan 15, 2017

In the meantime, if you need a workaround, you can do

iseq(x, y)::Bool = x == y

function g()
    x = Any[1]
    x = x[(!).(iseq.(x, 1))]
    iseq.(x, 0)
end

@KristofferC
Copy link
Sponsor Member

The original code is deprecated for broadcasted ! which works fine.

@tkelman
Copy link
Contributor

tkelman commented May 25, 2017

works fine

How so? I still see

function g()
    x = Any[1]
    x = x[(!).(x .== 1)]
    x .== 0
end
g()

return a 0-element Array{Any,1}

also with

julia> function g2()
           x = Any[1]
           x = x[.!(x .== 1)]
           x .== 0
       end
g2 (generic function with 1 method)

julia> g2()
0-element Array{Any,1}

@KristofferC
Copy link
Sponsor Member

Yeah, I goofed here.

@KristofferC KristofferC reopened this May 25, 2017
@vtjnash vtjnash added needs decision A decision on this change is needed and removed compiler:inference Type inference labels Jun 9, 2017
@vtjnash
Copy link
Sponsor Member

vtjnash commented Jun 9, 2017

we can simplify the original question to simply:

julia> [] .== 0
0-element Array{Any,1}

Which further simplifies to:

Core.Inference.return_type(==, (Any, Int)) === Bool

However, we haven't typically been willing to commit to forcing == to always return Bool, so this isn't expected to work.

@TotalVerb
Copy link
Contributor

Why do we return Array{Any, 1} instead of Array{Union{}, 1} in the empty case?

@nalimilan nalimilan added the triage This should be discussed on a triage call label Feb 17, 2018
@nalimilan
Copy link
Member Author

Are we OK with the current behavior of [] .== 0, or should it be changed to return Union{}[]?

@JeffBezanson
Copy link
Sponsor Member

In #17811 I was convinced to always use the inferred type instead of Union{}.

@nalimilan
Copy link
Member Author

Let's close this then.

@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Feb 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs decision A decision on this change is needed regression Regression in behavior compared to a previous version
Projects
None yet
Development

No branches or pull requests

9 participants