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

Splatting does not work inside [] for variable declarations #1964

Closed
AzamatB opened this issue May 14, 2019 · 10 comments · Fixed by #1977
Closed

Splatting does not work inside [] for variable declarations #1964

AzamatB opened this issue May 14, 2019 · 10 comments · Fixed by #1977
Labels
Type: Error Messages Can be fixed with better error message Type: Feature request

Comments

@AzamatB
Copy link

AzamatB commented May 14, 2019

Write

using JuMP
using Gurobi

model = Model(with_optimizer(Gurobi.Optimizer))

c = rand(2,3)

now the following

julia> @variable(model, X[axes(c)...])
ERROR: syntax: "..." expression outside call

does not work, while

julia> @variable(model, X[axes(c,1), axes(c,2)])
2×3 Array{VariableRef,2}:
 X[1,1]  X[1,2]  X[1,3]
 X[2,1]  X[2,2]  X[2,3]

does.

@AzamatB
Copy link
Author

AzamatB commented May 15, 2020

Seems like this issue was not addressed. Instead, an error message was added saying "do not use splatting".

Some explanation would be helpful for the motivation behind closing this issue. Otherwise, just silently closing issues like this really demotivates from filing them in the future.

@odow
Copy link
Member

odow commented May 15, 2020

Hi @AzamatB,

See the discussion in the pull request #1977.

There are some fundamental challenges that mean making splatting work here is very hard. In particular, splatting needs to happen at run-time, whereas we rewrite the contents inside the macros at compile time.

Essentially

@variable(model, x[A, B])

# becomes

x = ...
for a in A
    for b in B
        x[a, b] = add_variable(model)
    end
end

with the splatting, we don't know how many elements there are, so we can't preallocate x to be the correct size, and we can't write the correct number of for-loops.

I appreciate you opening the issue. Please feel free to open more in future. It helps to improve the quality of JuMP :)

@1ozturkbe
Copy link

@odow @mlubin I appreciate the nice warning, but what actually is the way to get around splatting?
eg. how to automatically generate anonymous variables with the same dimensions as a set of multi-dimensional arrays without splatting?
I know performance is one of the top goals for JuMP, but not supporting splatting makes programmatic generation of JuMP.Models difficult.

@mlubin
Copy link
Member

mlubin commented Dec 4, 2020

Splatting is restricted only in the macros; it's a parsing issue. If you want more flexibility, you should choose a container type (e.g., Array, DenseAxisArray, SparseAxisArray, [insert your favorite data structure]) and construct it with function calls that allow you to splat. See https://jump.dev/JuMP.jl/v0.21.5/containers/.

@1ozturkbe
Copy link

This is still not clear to me. I don't understand what you mean by "construct it with function calls that allow you to splat".
Otherwise I've tried so many combinations I can't remember all of them. Here's a few.

using JuMP
model = JuMP.Model()
dims = [Base.OneTo(5), Base.OneTo(3)] # let's pretend we don't know the dimensions
v = @variable(model, [dims...]) # doesn't work, as discussed above
v = @variable(model, dims) # obviously doesn't work, since it believes dims is a variable name
idxs = collect(Base.product([collect(s) for s in sizes]...))
v = @variable(model, [idxs]) # doesn't have desired behavior, since v has dimension 1. 
array = JuMP.Containers.DenseAxisArray(idxs, sizes...)
v = @variable(model, [array]) # doesn't work, since indexing is by v[Tuple], not v[Tuple...]

I am stumped. What is the right approach here?

@joehuchette
Copy link
Contributor

For example:

using JuMP
model = JuMP.Model()
dims = [Base.OneTo(5), Base.OneTo(3)]
array = JuMP.Containers.DenseAxisArray{JuMP.VariableRef}(undef, dims...)
for idx in eachindex(array)
    array[idx] = @variable(model)
end

@mlubin
Copy link
Member

mlubin commented Dec 4, 2020

Or if you want a plain Array:

sizes = [5, 3]
v = Array{JuMP.VariableRef}(undef, sizes...)
for i in eachindex(v)
   v[i] = @variable(model)
end

@1ozturkbe
Copy link

Ok, maybe I am being a pain in the neck, but this is precisely what I would prefer not to do. Because let's say I decided to name the variables:

dims = [Base.OneTo(5), Base.OneTo(3)]
array = JuMP.Containers.DenseAxisArray{JuMP.VariableRef}(undef, dims...)
for idx in eachindex(array)
    array[idx] = @variable(model, base_name = "c")
end

Now the issue is when I call variables_by_name(model, "c"), I get errors. I lose the ability to call all Array{VariableRef, 2} other than the global v, since under the hood the variables are scalar. Is there a way to preserve the structure?

@joehuchette
Copy link
Contributor

joehuchette commented Dec 4, 2020

Is variables_by_name defined in JuMP? How about just:

for idx in eachindex(array)
    array[idx] = @variable(model)
    set_name(array[idx], "c[$(join(Tuple(idx),","))]")
end

@1ozturkbe
Copy link

1ozturkbe commented Dec 4, 2020

variables_by_name is not defined, as far as I can tell.
@joehuchette that seems to work similarly to @variable(model, [1:5, 1:3], base_name = "c"), thanks!
@mlubin thanks for the insights as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Error Messages Can be fixed with better error message Type: Feature request
Development

Successfully merging a pull request may close this issue.

5 participants