disallow colons in variable index sets

[mlubin]

I feel like there's a mental block on the indexing with colon issue, hopefully this pushes us forward. Letting people index with colons as keys just seems like a terrible idea in any case, given the syntax confusion.

We can even merge and let it hang around for a release before we implement slicing.

Ref #643

[codecov-io]

Current coverage is 86.65%

Merging #782 into master will increase coverage by <.01%

@@             master       #782   diff @@
==========================================
  Files            18         18          
  Lines          4047       4052     +5   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           3506       3511     +5   
  Misses          541        541          
  Partials          0          0          

Powered by Codecov. Last updated by 8227200...33c184f

[joehuchette]

Why make this a generated function?

[mlubin]

Why make this a generated function?

This way the check gets lifted to compile time in most cases.

[joehuchette]

Only in the case with well-typed index sets, where the compiler should be
able to optimize away isa(foo, Colon) anyway. In the case of
heterogeneous arrays, this will potentially be much more expensive than a
runtime check.

On Thu, Jun 2, 2016 at 2:35 AM, Miles Lubin notifications@github.com
wrote:

Why make this a generated function?

This way the check gets lifted to compile time in most cases.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#782 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABTzUhAAVJFc6vPpAyc51J8JAvXBuAOdks5qHnmngaJpZM4Ir9K4
.

[mlubin]

What are you proposing instead, and why would it be faster than generated functions?

[joehuchette]

Pathological case:

using JuMP

type Foo{T} end

N = 100
I = [Foo{T} for T in 1:N]

m = Model()
@time @variable(m, x[I,I;true])

First run (i.e. compile time) takes 16 seconds on this branch and 0.5 seconds on master (this is on v0.5; v0.4 is much slower for both). Just make it a runtime check:

function coloncheck(args...)
    if any(t -> isa(t,Colon), args)
        return error("Colons not allowed as keys in JuMP containers.")
    else
        return nothing
    end
end

This gives roughly the same performance as current master.

[mlubin]

How about this version?

[joehuchette]

LGTM

[mlubin]

Did you check for regressions?

[joehuchette]

No regressions on my example case.