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

Add batched modification methods #3716

Merged
merged 22 commits into from Mar 28, 2024
Merged

Add batched modification methods #3716

merged 22 commits into from Mar 28, 2024

Conversation

joaquimg
Copy link
Member

@joaquimg joaquimg commented Mar 22, 2024

Modify problems bit by bit can be surprisingly slow in some solvers (like Xpress). Batch modifications like this solve the problems. I was working on a model that changes from 750s to 150s build+modify+solve time after using batch modifications.

rhs would be weird in plural as rhss, I don't know what to do about it.

Copy link

codecov bot commented Mar 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.41%. Comparing base (1aedb44) to head (48012e9).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3716      +/-   ##
==========================================
+ Coverage   98.37%   98.41%   +0.03%     
==========================================
  Files          43       43              
  Lines        5737     5813      +76     
==========================================
+ Hits         5644     5721      +77     
+ Misses         93       92       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/objective.jl Outdated Show resolved Hide resolved
src/variables.jl Outdated Show resolved Hide resolved
@odow odow changed the title Batch modification methods Add batched modification methods Mar 26, 2024
@odow odow mentioned this pull request Mar 27, 2024
13 tasks
src/objective.jl Outdated Show resolved Hide resolved
src/objective.jl Outdated Show resolved Hide resolved
src/objective.jl Outdated Show resolved Hide resolved
Copy link
Member

@blegat blegat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, only minor comments

src/objective.jl Outdated Show resolved Hide resolved
@pedroripper
Copy link

Hey everyone!

I tried to use this new dispatch for set_normalized_coefficient but got an error... here is a MWE

using JuMP
using HiGHS

model = Model(HiGHS.Optimizer)

@variable(model, x[1:3] >= 0)
@constraint(model, c[i in 1:3], x[i] == i)

constraint_coeffs = Vector{JuMP.ConstraintRef}()
variables = Vector{JuMP.VariableRef}()
coefficients = Vector{Float64}()

for i in 1:3
    push!(constraint_coeffs, c[i])
    push!(variables, x[i])
    push!(coefficients, 5)
end

set_normalized_coefficient(constraint_coeffs, variables, coefficients)

And when I am more specific about the type of the constraint reference vector, it works

using JuMP
using HiGHS

model = Model(HiGHS.Optimizer)

@variable(model, x[1:3] >= 0)
@constraint(model, c[i in 1:3], x[i] == i)

constraint_coeffs =  Vector{JuMP.ConstraintRef{JuMP.Model, MOI.ConstraintIndex{MOI.ScalarAffineFunction{Float64}, MOI.EqualTo{Float64}}, ScalarShape}}()
variables = Vector{JuMP.VariableRef}()
coefficients = Vector{Float64}()

for i in 1:3
    push!(constraint_coeffs, c[i])
    push!(variables, x[i])
    push!(coefficients, 5)
end

set_normalized_coefficient(constraint_coeffs, variables, coefficients)

@odow
Copy link
Member

odow commented Mar 28, 2024

@joaquimg I'll fix the error messages.

@pedroripper this is expected behavior because of the abstract Vector{ConstraintRef}.

@odow odow merged commit c0dca14 into master Mar 28, 2024
11 checks passed
@odow odow deleted the jg/batchmod branch March 28, 2024 22:40
@franckgaga
Copy link
Contributor

franckgaga commented Apr 4, 2024

@joaquimg @odow @blegat I just wanted to thank you for this excellent contribution, you are doing amazing work!

ModelPredictiveControl.jl uses OSQP.jl by default and it also has this issue. I raised an issue (#3566) in which I was suspecting a type instability for the bad performance. It was not the culprit at all. Using the new set_normalized_rhs and set_objective_coefficient methods with vectors drastically improved the performance ! 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants