Extend setvalue to JuMPArrays

[anriseth]

This commit enables us to run

@defVar(m, x[1:n])
setValue(x, x0)

for an array of numbers, x0.

Hope I put the code in the right place?

[mlubin]

This method only really makes sense for OneIndexedArray. Otherwise you could do:

@defVar(m, x[[:a,:b,:c], 1:2])
setValue(x, [1,2,3,4,5,6])

which doesn't make too much sense.

[anriseth]

Your example gives me the error

ERROR: `setValue` has no method matching setValue(::JuMPDict{Variable,2}, ::Array{Int64,1})

in v0.3.12. Maybe there have been some changes in v0.4?

It seems like my restriction to JuMPArrays means this does not work with variables that use symbols in indices.

[mlubin]

Oh, I see the method was for JuMPArray, try this:

@defVar(m, x[5:7, 1:2])
setValue(x, [1,2,3,4,5,6])

[anriseth]

Seems fine?

julia> @defVar(m, x[5:7, 1:2])
x[i,j] free for all i in {5,6,7}, j in {1,2}

julia> setValue(x, [1,2,3,4,5,6])
ERROR: dimensions must match
 in promote_shape at operators.jl:191
 in map at abstractarray.jl:1352
 in setValue at /home/asbjorn/.julia/v0.3/JuMP/src/JuMP.jl:267

[mlubin]

I should test before posting code. This code works without error:

julia> @defVar(m, x[4:5,3:4])
x[i,j] free for all i in {4,5}, j in {3,4}

julia> setValue(x, [1 2; 3 4])
2x2 Array{Nothing,2}:
 nothing  nothing
 nothing  nothing

I would argue that it's misleading to accept this as input if the variable x doesn't use one-based indexing. Hence OneIndexedArray.

Also returning an array of nothing is a bit strange. Anyway these should be small fixes, otherwise I like the syntax.

[anriseth]

I see from the build logs that getValue(x) returns an Array for that version of Julia+JuMP.

It runs fine on my setup, but maybe it's too outdated?
Julia 0.3.12-pre+3, JuMP 0.9.3

[mlubin]

Yes, on JuMP master getValue returns a julia Array when possible.

[mlubin]

Would also be good to add a test for the error case with mismatching dimensions.

[anriseth]

Ah, I understand what you mean by restricting it to OneIndexedArray now ...

map() returns Nothing: are you happy with adding an empty return-line at the end of the function instead?

Added dimension mismatch test as well.

[mlubin]

1. OneIndexedArray no longer exists as of [RFC] Remove OneIndexedArray #548 (sorry for the churn). Only need to worry about Array{Variable} now.
2. To avoid allocating an extra array of nothings, I think it makes sense to implement setValue as first checking that the sizes match and then just have an explicit loop over eachindex of the arrays. @joehuchette?

[joehuchette]

Yes, a loop like

for I in eachindex(x)
    setValue(set[I], val[I])
end
nothing

should do the trick.

[anriseth]

1. I changed the setValue(OneIndexedArray, ...) back to JuMPArray. Do you want to keep this, or remove it entirely?
2. Makes sense. I guess promote_shape isn't exactly what we're looking for, but the allocation of a shape tuple doesn't make much of a difference.

[mlubin]

I changed the setValue(OneIndexedArray, ...) back to JuMPArray. Do you want to keep this, or remove it entirely?

I'd rather remove this entirely. JuMPArray objects aren't indexed in the same way as julia arrays.

promote_shape seems to be reasonable here.

Note travis is failing on 0.4:

 Failure :: (line:-1) :: wrong exception was thrown
    Expression: setValue(y,[1:6])
      Expected: ErrorException
      Occurred: DimensionMismatch

May need to special-case that test for 0.3 and 0.4.

[anriseth]

I removed the JuMPArray call and added a 0.4 case.

[mlubin]

Shouldn't this be setValue(x, x0)?

[anriseth]

I removed setValue(::JuMPArray, ...) as you wanted, so it is no longer possible to run setValue(x, x0).

[anriseth]

Hmm. I see from the test logs that your Julia/JuMP version is not happy with calling .innerArray. Whilst my setup is not happy with setValue(x, x0) ...

[joehuchette]

Yep, this behavior changed in #548 about 16 hours ago :)

[mlubin]

If you're running with JuMP master, setValue(x, x0) will work.

[anriseth]

Hah, ok - I should learn to rebase on upstream master more often.

[joehuchette]

Looks good to me! Thanks. We can merge after CI passes.

[mlubin]

Thanks!