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

Macros should use add_variables & add_constraints rather than singular methods #1939

Closed
ndinsmore opened this issue Apr 9, 2019 · 13 comments
Labels
Status: Needs developer call This should be discussed on a monthly developer call Type: Performance
Milestone

Comments

@ndinsmore
Copy link
Contributor

I would like to advocate that the @constraint and @variable macro should be updated to use add_constraints and add_variables rather than add_constraints.(...) and add_variables.(...).

The issue here ends up being with solvers where adding constraints one by one is very slow, which is particularly true with direct_model, on a solver like Clp. Which with the caching bridge & jump-dev/Clp.jl#57 is 40x faster in benchmarking than the direct_model benchmark.

@joaquimg
Copy link
Member

joaquimg commented Apr 9, 2019

How do you think this should work in JuMP?
That would only work when adding multiple variables at once like @variable(model, x[1:10])?

@mlubin
Copy link
Member

mlubin commented Apr 9, 2019

This is easier to do for @variable than @constraint. MOI.add_constraints assumes that the constraints are homogeneous (http://www.juliaopt.org/MathOptInterface.jl/dev/apireference/#MathOptInterface.add_constraints), while @constraint doesn't require that.

@joaquimg
Copy link
Member

joaquimg commented Apr 9, 2019

At some point we discussed that his could be implemented in LQOI, JuMP would stay the way it is and LQOI could do some caching.

Some other interesting idea would be a mixed mode:

  1. Build the problem with caching optimizer
  2. push everything at once with add_variables and add_constraints with a specialized copy_to
  3. the start using direct mode for modifications and so on

By the way,
if we used a specilized copy_to from caching to linquad you problem would be solved?

@ndinsmore
Copy link
Contributor Author

ndinsmore commented Apr 9, 2019

It looks likes constraints already branches for the "plural version" . Besides the names I don't see why that is actually neccisary. Even if you are only adding one contraint you can still use add_constraints
https://github.com/JuliaOpt/JuMP.jl/blob/903821b15f6bc5f69486a3eec70d0004d30f01bc/src/macros.jl#L593-L598

We could always add a non-homogenous fall back to add_constraints in MOI that just uses add_constraint.(...) . That might actually still be faster because the optimizer could like deal with the type union better.

@ndinsmore
Copy link
Contributor Author

ndinsmore commented Apr 9, 2019

No need for the speciallized copy_to, with:
JuliaOpt/LinQuadOptInterface.jl#101
jump-dev/Clp.jl#57
is already very fast by using the plural add_ methods.

You can see the benefit to the optimize! call in the following benchmark from jump-dev/Clp.jl#57

Benchmarking code:.

using JuMP
using Clp
using GLPK
using BenchmarkTools
using Random
using SparseArrays
using Profile

function  build_model(solver, sp_mat; opt_options::NamedTuple=NamedTuple())
    model = Model(with_optimizer(solver.Optimizer; opt_options...))
    # model = Model(with_optimizer(solver.Optimizer; opt_options...))
    num_variables=size(sp_mat,2)
    # display(Matrix(sp_mat))
    vars=@variable(model,0<=vars[i=1:num_variables]<=0)

    cons=@constraint(model,cons,sp_mat * vars .== 0)
    return model
end
function  build_model_direct(solver, sp_mat; opt_options::NamedTuple=NamedTuple())
    model = JuMP.direct_model(solver.Optimizer( ;opt_options...))
    # model = Model(with_optimizer(solver.Optimizer; opt_options...))
    num_variables=size(sp_mat,2)
    # display(Matrix(sp_mat))
    vars=@variable(model,0<=vars[i=1:num_variables]<=0)

    cons=@constraint(model,cons,sp_mat * vars .== 0)
    return model
end
num_vars=10000
num_cons=10000
density=.001
rng=Random.MersenneTwister(1)
sp_mat=sprand(rng,num_cons,num_vars,density)

direct_build_clp = @benchmark model=build_model_direct(Clp,sp_mat,opt_options=(LogLevel=1,MaximumSeconds=10.0))
build_clp = @benchmark model=build_model(Clp,$sp_mat,opt_options=(LogLevel=1,MaximumSeconds=10.0))
opt_clp =@benchmark optimize!(model) setup=(model=build_model(Clp,$sp_mat,opt_options=(LogLevel=0,MaximumSeconds=10.0)))

Master:

julia> direct_build_clp = @benchmark model=build_model_direct(Clp,sp_mat,opt_options=(LogLevel=1,MaximumSeconds=10.0))
BenchmarkTools.Trial: 
  memory estimate:  1.56 GiB
  allocs estimate:  959800
  --------------
  minimum time:     2.868 s (6.70% GC)
  median time:      3.095 s (7.84% GC)
  mean time:        3.095 s (7.84% GC)
  maximum time:     3.323 s (8.83% GC)
  --------------
  samples:          2
  evals/sample:     1

julia> build_clp = @benchmark model=build_model(Clp,$sp_mat,opt_options=(LogLevel=1,MaximumSeconds=10.0))
BenchmarkTools.Trial: 
  memory estimate:  30.32 MiB
  allocs estimate:  407369
  --------------
  minimum time:     50.563 ms (10.68% GC)
  median time:      68.254 ms (15.08% GC)
  mean time:        71.712 ms (16.95% GC)
  maximum time:     187.390 ms (57.66% GC)
  --------------
  samples:          70
  evals/sample:     1

julia> opt_clp =@benchmark optimize!(model) setup=(model=build_model(Clp,$sp_mat,opt_options=(LogLevel=0,MaximumSeconds=10.0)))
BenchmarkTools.Trial: 
  memory estimate:  43.94 MiB
  allocs estimate:  900321
  --------------
  minimum time:     2.318 s (1.52% GC)
  median time:      2.323 s (1.85% GC)
  mean time:        2.354 s (2.92% GC)
  maximum time:     2.421 s (5.29% GC)
  --------------
  samples:          3
  evals/sample:     1

With above PRs:

julia> direct_build_clp = @benchmark model=build_model_direct(Clp,sp_mat,opt_options=(LogLevel=1,MaximumSeconds=10.0))
BenchmarkTools.Trial: 
  memory estimate:  1.56 GiB
  allocs estimate:  979800
  --------------
  minimum time:     3.035 s (9.45% GC)
  median time:      3.035 s (8.12% GC)
  mean time:        3.035 s (8.12% GC)
  maximum time:     3.036 s (6.79% GC)
  --------------
  samples:          2
  evals/sample:     1

julia> build_clp = @benchmark model=build_model(Clp,$sp_mat,opt_options=(LogLevel=1,MaximumSeconds=10.0))
BenchmarkTools.Trial: 
  memory estimate:  30.32 MiB
  allocs estimate:  407369
  --------------
  minimum time:     53.355 ms (12.94% GC)
  median time:      67.650 ms (15.54% GC)
  mean time:        69.863 ms (17.19% GC)
  maximum time:     160.476 ms (60.86% GC)
  --------------
  samples:          72
  evals/sample:     1

julia> opt_clp =@benchmark optimize!(model) setup=(model=build_model(Clp,$sp_mat,opt_options=(LogLevel=0,MaximumSeconds=10.0)))
BenchmarkTools.Trial: 
  memory estimate:  32.56 MiB
  allocs estimate:  780344
  --------------
  minimum time:     116.185 ms (15.59% GC)
  median time:      163.528 ms (22.02% GC)
  mean time:        170.163 ms (26.51% GC)
  maximum time:     284.184 ms (54.23% GC)
  --------------
  samples:          18
  evals/sample:     1

@joaquimg
Copy link
Member

joaquimg commented Apr 9, 2019

The specialized copy_to I am talking about is the one copying data from the CachingOptimizer to the LinQuadModel.

@ndinsmore
Copy link
Contributor Author

@joaquimg do you have a benchmark that would indicate how fast copy_to should be? Is there still a lot of low hanging fruit?

@joaquimg
Copy link
Member

Nope.
You are trying to build the model as fast as possible, right?
And maybe your problem is due to the fact that passing multiple constraints is better than one by one for the Clp low level API.
I am just suggesting that one possible alternative is:
Do not use direct mode, use a caching optimizer to hold all the JuMP that is the simplest possible format for JuMP.
Then convert all that data with one single efficient function into large matrices and pass the large matrices to Clp. You might get faster conversion from JuMP->Clp low level format and a single call to the Clp add_rows function which is fast.
I don't know if this is better, just saying it might be.

About changing the macros,
if you can make the necessary modification and it still work for all the cases with proper fallbacks and benchmarks are better, then there is reason not to do it IMO.

@joaquimg
Copy link
Member

Since you are interested in the performance issues, you might want to look at this one: #1905
There are already some suggestions like JuliaOpt/LinQuadOptInterface.jl#98
And the julia issue you got involved: JuliaLang/julia#31199

@blegat
Copy link
Member

blegat commented Apr 10, 2019

We could create an array of the ScalarVariable in the macro and then add

function add_variables(model::JuMP.Model, variables::Vector{ScalarVariable}, names::Vector{String})
    variable_indices = MOI.add_variables(model, length(variables))
    # set bounds and names...
end

@ndinsmore
Copy link
Contributor Author

@joaquimg

And maybe your problem is due to the fact that passing multiple constraints is better than one by one for the Clp low level API.

jump-dev/MathOptInterface.jl#696 made it so that the default copy_to now uses add_constraints and for Clp at least it enables the 30x speed increase above.

This is the direct_model side of the same change that was made to MOI.

@joaquimg
Copy link
Member

Nice! I support this!

@odow
Copy link
Member

odow commented Oct 12, 2021

See #2748 for an investigation into this.

@odow odow added the Status: Needs developer call This should be discussed on a monthly developer call label Oct 27, 2021
@odow odow closed this as completed Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs developer call This should be discussed on a monthly developer call Type: Performance
Development

No branches or pull requests

5 participants