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.

Sign up for GitHub

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

Jump to bottom

Delete constraint(s), add and remove multiple warmstarts #121

Closed
wants to merge 8 commits into from
Closed

Delete constraint(s), add and remove multiple warmstarts #121

wants to merge 8 commits into from

Conversation

aritrasep
Copy link

No description provided.

joehuchette
joehuchette reviewed Mar 21, 2017
View reviewed changes
src/cpx_model.jl Outdated
@@ -174,6 +174,57 @@ function set_warm_start!(model::Model, indx::IVec, val::FVec, effortlevel::Integ
end
end

function add_warm_start!(model::Model, x::Vector{Vector{Float64}}, efforts::Vector{Cint})
beg::Vector{Cint}, inds::Vector{Cint}, vals::Vector{Cdouble} = Cint[], Cint[], Cdouble[]
count::Int64 = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Type annotation here shouldn't be necessary.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry about that. Its just an old habit.

src/cpx_model.jl Outdated
@@ -174,6 +174,57 @@ function set_warm_start!(model::Model, indx::IVec, val::FVec, effortlevel::Integ
end
end

function add_warm_start!(model::Model, x::Vector{Vector{Float64}}, efforts::Vector{Cint})
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not crazy about the naming here. Probably better to write this as set_warm_starts!.

Copy link
Author

Choose a reason for hiding this comment

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

Sure

@joehuchette
Copy link
Contributor

Sorry for the confusion, I meant set_warm_starts! (note the plural "s").

@mlubin
Copy link
Member

mlubin commented Mar 21, 2017

Why not follow the CPLEX terminology for these wrappers and call them mip starts instead of warm starts?

@mlubin
Copy link
Member

mlubin commented Mar 21, 2017

Make sure to account for renamings in other parts of the code, e.g., CplexSolverInterface.jl.

@aritrasep
Copy link
Author

@mlubin Actually, I haven't tested it using JuMP.jl or MathProgBase.jl. I am using CPLEX.jl directly / independently to create, modify and solve a mip. So, these functions have not been wrapped for MathProgBase.jl.

@mlubin
Copy link
Member

mlubin commented Mar 22, 2017

I thought you had also renamed set_warm_start! to set_mip_start! (which you should do for consistency). In this case you also need to update the other parts of the code that call set_warm_start! to avoid breaking CPLEX for people who do use it through JuMP or MathProgBase.

@aritrasep
Copy link
Author

I didn't rename the previous set_warm_start! to add_mip_start! so that it doesn't break if used through MathProgBase or JuMP. I can change the previous set_warm_start! as well to add_warm_start!, however there are some conflicts in the way the previous set_warm_start! is implemented.

@mlubin
Copy link
Member

mlubin commented Mar 22, 2017

You have two options:

  1. New functions are called set_warm_starts! and set_warm_start! is unchanged
  2. New functions are called set_mip_starts! and set_warm_start! is renamed to set_mip_start!

@odow odow mentioned this pull request Sep 23, 2020
Merged
@odow odow closed this in #316 Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joehuchette joehuchette joehuchette left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aritrasep @joehuchette @mlubin