Adding JuMPKey as elements of keys(::JuMPArray) and keys(::JuMPDict)

[IssamT]

I have added JuMPKey for JuMPArray as suggested in #646 . And I would like to add it for JuMPDict as well. Do you know how to make a small example returning JuMPDict ?

[mlubin]

@IssamT, use dependent indices or conditions,

@variable(m, x[i=1:N,j=1:i])
@variable(m, x[i=1:N,j=1:N; j <= i])

[codecov-io]

Current coverage is 87.04% (diff: 93.75%)

Merging #853 into master will increase coverage by 0.14%

@@             master       #853   diff @@
==========================================
  Files            18         18          
  Lines          4494       4507    +13   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           3905       3923    +18   
+ Misses          589        584     -5   
  Partials          0          0          

Powered by Codecov. Last update b5225ba...38e0d98

[IssamT]

Thanks for the example.
I suggest one possible implementation of JuMPKey with JuMPDict it in the last commit.

I have a small question : why are some functions of JuMPArray in the file JuMPContainer.jl and others in JuMPArray.jl?

[joehuchette]

No particular reason. The files are a bit disorganized, as the code has undergone a few serious rewrites in the past. Don't feel obligated to keep them structured as-is.

[joehuchette]

This looks pretty good to me. Can you test to make sure there are no accidental performance penalties? Also, could you add some tests?

[IssamT]

I got some weird results on this test.

set1 = 2:101
set2 = 2001:2100
m = Model()

# @variable(m, x[set1, set2], Bin) # For JuMPArray
@variable(m, x[i in set1, j in set2; i<j], Bin) # For JuMPDict
@objective(m , Max, sum{sum{x[e,p], e in set1}, p in set2})
solve(m)
sol = getvalue(x)

for i in 1:20
    @time begin
        for i in keys(sol)
        end
    end
end

With JuMPArray:
There are no noticeable difference between using JuMPKey or not.

With JuMPDict:
Using JuMPKey is about 14 times slower. It is what I get when using the previous commit and in particular these:

immutable JuMPKey{T<:Tuple}
    jk::T
end

function Base.next(it::JDKeyIterator, k)
    n = next(it.x, k)
    JuMPKey(n[1]), n[2]
end

---

I didn't understand the reason so I tried to check different versions. Using these:

immutable JuMPKey{T<:Tuple}
    jk::T
end

function Base.next(it::JDKeyIterator, k)
    n = next(it.x, k)
    n[1], n[2]
end

There is no overhead at all, so I understood that the problem is in JuMPKey.

---

Now using these:

immutable JuMPKey
    jk::Tuple
end

function Base.next(it::JDKeyIterator, k)
    n = next(it.x, k)
    JuMPKey(n[1]), n[2]
end

The iteration is about 3 to 4 times FASTER compared to the commit before my patch (without JuMPKey). I don't understand why... maybe immutable types are more efficient when non parameterized? I would be glad if anyone can provide an explanation of this weird "julia" behavior...

[IssamT]

I have reproduced the issue with plain julia Dict.

JuliaLang/julia#18765

[joehuchette]

As mentioned in the issue you opened, adding the type parameter to

immutable JuMPKey{T<:Tuple}
    jk::T
end

allows the compiler to reason about the concrete type of jk and use it to (hopefully) generate more efficient code. In your other version, it is only able to infer that jk is an (abstract) Tuple, rather than, say, a Tuple{Int,Int}.

[IssamT]

Just to be sure we speak about the same thing, what I find weird is that using this

immutable JuMPKey{T<:Tuple}
    jk::T
end

is much slower than using this

immutable JuMPKey
    jk::Tuple
end

I was expecting the opposite based on the same explanation @joehuchette has provided in his last comment.

I believe this weird behavior is due to the fact we are not allowing the compiler to reason with concrete types everywhere since we have types declared with Any like

tupledict::Dict{NTuple{N,Any},T}
meta::Dict{Symbol,Any}

I am afraid this issue is not only related to JuMPKey, and other functions might also be impacted by this, though I haven't got time yet to do more tests.

[joehuchette]

Think of T<:Tuple as a wildcard that matches any type T that's a subtype of Tuple. So you can have parameterized versions of your first definition that functionally act as

type JuMPKey
    jk::Tuple{Int,Int}
end

In your second definition, the type of the field jk is always the abstract type Tuple, which is strictly less informative to the compiler than the first version.

[IssamT]

I totally agree, but this

type JuMPKey
    jk::Tuple{Int,Int}
end

m̶u̶s̶t̶(Edit : should) be better than this (in term of performance)

type JuMPKey
    jk::Tuple
end

am I wrong?

[joehuchette]

Oh sorry, I had the two cases flipped in my mind. Yes, it is strange to me that the unparameterized version is faster...

[IssamT]

My guess is that julia's compiler isn't, yet, at the level of providing always improvements when you change some of your types from abstract to concrete, while keeping others abstract...

[mlubin]

This is stale, willing to consider new PRs