Closed blegat closed 3 years ago
OMG, thank you @blegat!
@tweisser!
@blegat @tweisser Thank you!
Merging #149 into master will decrease coverage by
0.76%
. The diff coverage is75.58%
.
@@ Coverage Diff @@
## master #149 +/- ##
==========================================
- Coverage 75.99% 75.23% -0.77%
==========================================
Files 15 16 +1
Lines 3183 3359 +176
==========================================
+ Hits 2419 2527 +108
- Misses 764 832 +68
Impacted Files | Coverage Δ | |
---|---|---|
src/Alpine.jl | 100.00% <ø> (ø) |
|
src/heuristics.jl | 36.23% <16.12%> (-0.70%) |
:arrow_down: |
src/matrix_opt_interface.jl | 28.00% <28.00%> (ø) |
|
src/operators.jl | 73.42% <43.33%> (-0.36%) |
:arrow_down: |
src/utility.jl | 48.87% <49.54%> (+0.96%) |
:arrow_up: |
src/multi.jl | 60.68% <62.50%> (-3.02%) |
:arrow_down: |
src/presolve.jl | 78.33% <66.66%> (-0.80%) |
:arrow_down: |
src/algorithm.jl | 76.19% <81.81%> (-0.35%) |
:arrow_down: |
src/bounds.jl | 82.64% <83.33%> (-3.65%) |
:arrow_down: |
src/moi_function2expr.jl | 86.20% <86.20%> (ø) |
|
... and 21 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 72814dd...f2735ff. Read the comment docs.
I suggest
function MOI.add_constraint(model::Optimizer, f::Union{MOI.ScalarAffineFunction, MOI.ScalarQuadraticFunction}, set)
push!(model.constr_expr_orig, _moi_function_to_expr(func))
push!(model.constraint_bounds_orig, MOI.NLPBoundsPair(_lower(set), _upper(set)))
push!(m.constr_structure, f isa MOI.ScalarAffineFunction ? :generic_linear : :generic_nonlinear)
return MOI.ConstraintIndex{typeof(f), typeof(set)}(length(model.constr_expr_orig))
end
@blegat While disabling the failing tests, its probably good to keep track of the ones disabled so that we can fix them later. The reason is I had disabled a few tests too as the convergence checks can vary from version of MIP solvers. Will need to work on filtering out inconsistent tests.
I agree, I'm keeping track of the list of them and we post a list. I don't know yet if it's a regression from Pavito or Alpine though
@blegat that's great!
The PR is ready to be merged, I have left some issues that might be good to close before releasing: https://github.com/lanl-ansi/Alpine.jl/issues/150, https://github.com/lanl-ansi/Alpine.jl/issues/151 and https://github.com/lanl-ansi/Alpine.jl/issues/152.
I'd suggest to first merge this PR and close these issues as separate PR so that people can already try this version while these issues are resolved and report or fix any issue they might encounter. This approached worked well for https://github.com/JuliaOpt/Pavito.jl/pull/21 as it allowed @tasseff to make two PRs: https://github.com/JuliaOpt/Pavito.jl/pull/26, https://github.com/JuliaOpt/Pavito.jl/pull/29 fixing some issues with the MOI wrapper.
Bump :)
@blegat and @tweisser This has been merged and thanks a bunch for your help in this migration. There are a few checks in runtests.jl which need to be fixed, which I shall do it on a separate PR, in addition to other open issues.
The following branch already a branch that started an MOI port https://github.com/lanl-ansi/Alpine.jl/tree/moi-wrapper. In addition this branch also brings several improvement to Alpine but it is still a WIP. We thought with @tweisser that it could be possible to make an MOI port that was less ambitious to provide MOI support to Alpine before this other branch is finished. So the goal of this PR is to try to make the minimal changes of Alpine while switching from MPB to MOI. The behavior of Alpine before and after this PR should be as close as possible, further changes that are not strictly necessary for the port should come in a later PR.