-
Notifications
You must be signed in to change notification settings - Fork 62
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
Fixes for julia 1.0 #184
Fixes for julia 1.0 #184
Conversation
required fixing conflicts in cpx_solve.jl
conic tests were disabled since MPB still use speye. ccall_intercept were temporarily disabled.
Now works, tested in Xpress and Gurobi |
src/CPLEX.jl
Outdated
if VERSION >= v"0.7.0-DEV.3382" | ||
using SparseArrays | ||
using LinearAlgebra | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also use Compat.XXX
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given the guideline update on discourse, I think we can remove this all-together and use 1.0-compatible features only
So removing the version check and no Compat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matbesancon, but those are needed for 0.7 and above.
Thank you guys for updating this package! |
Updated: (sorry, I don't want to add other comments, so I'm updating this one everytime I have news) Now I can start to optimize a model but still there are problems with CPLEX callbacks for lazy constraints: In
Can you tell me if it's correct? |
src/CPLEX.jl
Outdated
include("JuMPfunctions.jl") | ||
end | ||
end | ||
# if isdir(Pkg.dir("JuMP")) && VERSION < v"0.6-" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to delete this chunk of code and remove JuMPfunctions.jl
.
src/CplexSolverInterface.jl
Outdated
setbranchcallback!(m::CplexMathProgModel,f) = (m.branchcb = f) | ||
setincumbentcallback!(m::CplexMathProgModel,f) = (m.incumbentcb = f) | ||
setinfocallback!(m::CplexMathProgModel,f) = (m.infocb = f) | ||
MathProgBase.setinfocallback!(m::CplexMathProgModel,f) = (m.infocb = f) | ||
|
||
function cbgetmipsolution(d::CplexCallbackData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The the cb
functions also need a MathProgBase.
prefix if they're implementing the MPB API.
src/CplexSolverInterface.jl
Outdated
@@ -615,10 +616,10 @@ function setmathproglazycallback!(model::CplexMathProgModel) | |||
set_param!(model.inner.env, "CPX_PARAM_MIPCBREDLP", 0) | |||
set_param!(model.inner.env, "CPX_PARAM_PRELINEAR", 0) | |||
set_param!(model.inner.env, "CPX_PARAM_REDUCE", CPX_PREREDUCE_PRIMALONLY) | |||
cpxcallback = cfunction(mastercallback, Cint, (Ptr{Void}, Ptr{Void}, Cint, Ptr{Void}, Ptr{Cint})) | |||
cpxcallback = cfunction(mastercallback, Cint, (Ptr{Nothing}, Ptr{Nothing}, Cint, Ptr{Nothing}, Ptr{Cint})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cfunction
(here and below)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cfunction
does not exist in Julia 1.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was exactly the point of my comment :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry Miles, I thought you were making the changes, I didn't understand was only a comment. -.-"
It's sunday, I'm working, please apologize.
src/CplexSolverInterface.jl
Outdated
@@ -362,7 +363,7 @@ end | |||
|
|||
function cbgetmipsolution(d::CplexCallbackData, sol::Vector{Cdouble}) | |||
@assert d.state == :MIPSol | |||
stat = @cpx_ccall(getcallbacknodex, Cint, (Ptr{Void},Ptr{Void},Cint,Ptr{Cdouble},Cint,Cint), | |||
stat = @cpx_ccall(getcallbacknodex, Cint, (Ptr{Nothing},Ptr{Nothing},Cint,Ptr{Cdouble},Cint,Cint), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Cvoid
instead of Nothing
when referring to the C type.
src/cpx_common.jl
Outdated
end | ||
end | ||
|
||
const GChars = Union{Cchar, Char} | ||
const IVec = Vector{Cint} | ||
const FVec = Vector{Cdouble} | ||
const CVec = Vector{Cchar} | ||
const CoeffMat = Union{Matrix{Cdouble}, SparseMatrixCSC{Cdouble}} | ||
const CoeffMat = Union{AbstractArray{Cdouble, 2}, SparseMatrixCSC{Cdouble}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AbstractMatrix
would be more concise?
deps/build.jl
Outdated
@@ -15,7 +15,8 @@ function write_depsfile(path) | |||
end | |||
end | |||
|
|||
if is_apple() | |||
if (VERSION >= v"0.7.0-DEV.3382" && Sys.isapple()) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if REQUIRE file specifies julia version at 0.7, we don't need the VERSION >= v"0.7.0-DEV.3382"
conditions.
@static
can be added in front of the condition too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I thought we wanted to have at least one version of CPLEX.jl that works for both 0.6 and 1.0 (and 0.7) for the transition. No?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, if that's still the case forget what I just wrote :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still can use @static
for all these conditions though
deps/build.jl
Outdated
@@ -36,7 +39,8 @@ for v in reverse(cpxvers) | |||
end | |||
|
|||
wincpxvers = ["126","1261","1262","1263","127","1270","1271","128","1280"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const
in front of wincpxvers
src/CPLEX.jl
Outdated
@@ -6,7 +6,8 @@ module CPLEX | |||
using Libdl | |||
end | |||
|
|||
if is_apple() | |||
if (VERSION >= v"0.7.0-DEV.3382" && Sys.isapple()) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing here @static
and no version check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please test the callbacks by running the JuMP 0.18 tests with CPLEX.
src/cpx_common.jl
Outdated
end | ||
ret | ||
println("some function is called using cpx_ccall_intercept") | ||
# ccall(:jl_exit_on_sigint, Nothing, (Cint,), convert(Cint,0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is blocking the rest of the PR, we can restore it for 0.6 and leave it as a TODO to write a compatible version for 1.0.
@mlubin, did you mean the file |
Yes.
What are they? They should be fixed regardless of whether they're in JuMP or CPLEX.jl. |
src/cpx_common.jl
Outdated
# TODO fix for 0.7 and above (using cpx_call instead temporarily) | ||
if VERSION < v"0.7.0-DEV.3382" | ||
f = "CPX$(func)" | ||
println(f) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover debugging code?
These are the errors I have now for the test/callback.jl https://justpaste.it/4x67j |
@IssamT Could you try JuliaOpt/MathProgBase.jl#227? |
Apparently this doesn't work. from JuMP's callback.jl : @test macroexpand(:(@lazyconstraint(cb, sum(x) <= 0, badkwarg=true))).head == :error |
All callback tests are passing for 0.6 and 1.0 except the test in line 307 of callback.jl @test solve(mod, suppress_warnings=true) == :UserLimit it returns |
|
Thanks. Fixed. So the only issue remaining is the test with |
This is a continuation on the nice work started by @matbesancon in #179. Tests are now passing for 0.6 and 1.0. Still some work to do:
speye
.cpx_ccall_intercept
. I temporarily replaced it withcpx_ccall
.