Slow behavior for setting or deleting variable bounds

[dlcole3]

JuMP.set_lower_bound and JuMP.delete_lower_bound (and their corresponding upper bound functions) are much slower on Plasmo OptiGraphs than in JuMP. In the following code, the Plasmo OptiGraph tests about 100x slower on my laptop:

using JuMP, Plasmo

m = Model()
@variable(m, x[1:10000], Bin)

g = OptiGraph()
@optinode(g, n)
@variable(n, y[1:10000], Bin)

@time begin
    unset_binary.(x)
    set_upper_bound.(x, 1)
    set_lower_bound.(x, 0)
end

@time begin
    unset_binary.(y)
    set_upper_bound.(y, 1)
    set_lower_bound.(y, 0)
end

I am not yet sure why this is, but I will try to take a closer look later this week. If anyone else knows already where this bottleneck might be, please reply.

[dlcole3]

Putting some additional findings here for reference:

I think this is caused in part by Plasmo using the constraint reference often times where the corresponding JuMP functions use a constraint index. For instance, setting the lower bound uses _moi_add_constraint here which I believe is defined here. In contrast, Plasmo uses _moi_set_nv_lower_bound here which uses _moi_add_node_constraint here. In JuMP, this function appears to be more elegant and uses just the index rather than the constraint reference.

As a further example, unset_binary uses JuMP.delete (see here) which only uses a constraint index in JuMP (see here). In contrast, Plasmo uses the constraint reference directly, not the index (see here).

@jalving could this be the reason for the slow behavior I talk about above? I am guessing that Plasmo uses the constraint reference because each of these elements (variables or constraints) might belong to multiple OptiGraphs. Does the index change depending on which owning subgraph you consider?

[dlcole3]

After doing some more searching, it looks like these lines are taking the most time for unset_binary. I am not sure yet how to speed these up, but I will think about it some more. Any suggestions are welcome.

[jalving]

Nice catch, using findall is most definitely a problem for performance. We will need to fix it for variables as well. An alternative is to not use a list to store element constraints and just use a dictionary for this as well.

[dlcole3]

Nice catch, using findall is most definitely a problem for performance. We will need to fix it for variables as well. An alternative is to not use a list to store element constraints and just use a dictionary for this as well.

Sounds good. I think a dictionary might be the way to go if it won't slow down other functions to have a dictionary rather than a vector. For reference, when I profiled some code that used the JuMP.delete! function, it was about 60-70% the findall command and the other 30-40% was the deleteat! call to remove it from the vector.