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

indicator to SOS bridge #877

Merged
merged 40 commits into from
Nov 26, 2019
Merged

indicator to SOS bridge #877

merged 40 commits into from
Nov 26, 2019

Conversation

matbesancon
Copy link
Contributor

@matbesancon matbesancon commented Sep 11, 2019

Based on the comment on indicator constraints in the SCIP documentation:

This constraint is equivalent to a linear constraint ax−s ≤ b and an SOS1 constraint on z and s (at most one should be nonzero). In the indicator context we can, however, separate more inequalities.

@codecov-io
Copy link

codecov-io commented Sep 11, 2019

Codecov Report

Merging #877 into master will decrease coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #877      +/-   ##
==========================================
- Coverage   95.22%   95.15%   -0.07%     
==========================================
  Files          84       86       +2     
  Lines        8978     9167     +189     
==========================================
+ Hits         8549     8723     +174     
- Misses        429      444      +15
Impacted Files Coverage Δ
...c/Bridges/Constraint/indicator_activate_on_zero.jl 100% <ø> (ø)
src/Test/UnitTests/basic_constraint_tests.jl 98.21% <ø> (ø) ⬆️
src/Bridges/Constraint/Constraint.jl 96.15% <100%> (+0.15%) ⬆️
src/Bridges/Constraint/indicator_sos.jl 100% <100%> (ø)
src/constraints.jl 88.88% <100%> (ø) ⬆️
src/Bridges/Objective/bridge.jl 30% <0%> (-30%) ⬇️
src/Bridges/Constraint/bridge.jl 75% <0%> (-25%) ⬇️
src/Bridges/Constraint/norm_to_lp.jl 90.78% <0%> (-9.22%) ⬇️
src/Bridges/debug.jl 96.74% <0%> (-3.26%) ⬇️
src/Bridges/Variable/free.jl 96.96% <0%> (-3.04%) ⬇️
... and 5 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 1d4f0d3...5d3ab8c. Read the comment docs.

@matbesancon
Copy link
Contributor Author

this should be good for review

@matbesancon
Copy link
Contributor Author

ping @blegat @mlubin

@odow
Copy link
Member

odow commented Sep 12, 2019

ping @blegat @mlubin

I believe they're on vacation.

Have you tried this with some solvers? E.g., GLPK?

@matbesancon
Copy link
Contributor Author

thanks that's why it's all quiet around here!

Not yet I wanted to be sure the whole thing was not off, I'll give it a try

@odow
Copy link
Member

odow commented Sep 12, 2019

Seems okay to me. The only style question is whether to include it in bridges/constraint/indicator or in a separate file.

@matbesancon
Copy link
Contributor Author

GLPK does not seem to support SOS constraints. The other indicator bridge defined is for activated on 0 => activated on 1, it's not awfully big

@odow
Copy link
Member

odow commented Sep 12, 2019

GLPK does not seem to support SOS constraints.

Oh. I always forget that. Gurobi then?

@matbesancon
Copy link
Contributor Author

Cbc seems ok, I'm trying to use only the FOSS ones for tests

@matbesancon
Copy link
Contributor Author

So, it seems tricky, I'm not sure this is supposed to behave as ignoring the bridge, needs more investigation

@matbesancon
Copy link
Contributor Author

There seems to be a problem, but I haven't figured out what exactly. I still haven't figured how to check exactly the newly formulated problem

@odow
Copy link
Member

odow commented Sep 13, 2019

I still haven't figured how to check exactly the newly formulated problem

Try saving to MathOptFormat?

@matbesancon
Copy link
Contributor Author

matbesancon commented Sep 13, 2019 via email

`EqualTo` constraints are handled without a bound constraint:
``z \\in \\mathbb{B}, z == 1 \\implies f(x) == b`` is reformulated as:
``z \\in \\mathbb{B}, w \\text{ free}, f(x) + w == b, SOS1(w, z)``.

Copy link
Member

Choose a reason for hiding this comment

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

Why not always add a free w so that it works for any indicator constraint (also MOI.Interval, ...) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we have a free fallback with all constraints but LessThan, GreaterThan, otherwise we add a free, which has to be re-bounded by two variables for LP& MILP solvers

Copy link
Member

Choose a reason for hiding this comment

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

Not sure I understood "which has to be re-bounded by two variables"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry yes, if we create the variable w free, it will be replaced by w+ >= 0, w- >= 0, w = w+ + w- instead of creating a bounded w right away

Copy link
Member

Choose a reason for hiding this comment

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

That's also the case for SDP solvers with the Variable.FreeBridge. Which LP solver does not support free variables ? This transformation is done internally ?
For a solver supporting free variables such constraints may increase the size of the problem, we should probably have informative constraints but that's out of the scope of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which LP solver does not support free variables ? This transformation is done internally ?

All solvers (that I am aware of) support free variables, but the transformation needs to be done internally somehow. The simplex algorithm does require positive variables

Copy link
Member

Choose a reason for hiding this comment

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

If you want to use the standard form you have equality constraints and nonnegative variables but then the dual has free variables. Some solvers might solve the dual using the simplex in which case they need all variables to be free. Although that's the textbook simplex so I don't know whether solvers might use a more sophisticated one supporting mixing free and non-free ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you want to use the standard form you have equality constraints and nonnegative variables but then the dual has free variables.

My understanding is that the standard form is a form A x <= b, x >= 0 transformed in Ax + s = b, x, s >= 0. You need to have basic columns in the equality, so starting from a generic A x = b, it will need to change the variable system in [A; -A] x <= [b; -b] and then add slacks, in order to have as many basic columns as constraints.

@@ -53,3 +53,92 @@ include("../utilities.jl")
@test t.coefficient ≈ 1.0
end
end

Copy link
Member

Choose a reason for hiding this comment

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

We should also have a test with basic_constraint_test (see #750) and a test against indicator1, indicator2 or indicator3 (https://github.com/JuliaOpt/MathOptInterface.jl/blob/98a0e472e1757db14b849bbba8fe84597856def2/src/Test/intlinear.jl#L609-L611) with test_model_equals (see #820)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't manage to make that work (yet)

@matbesancon
Copy link
Contributor Author

@blegat I can't figure out what is off here. Any tips on a debugging process for that?

@blegat
Copy link
Member

blegat commented Oct 4, 2019

I can't figure out what is off here. Any tips on a debugging process for that?

You mean for making basic_constraint_tests working ? What is the issue you are facing ? Can you share an error message ?

@matbesancon
Copy link
Contributor Author

The constraints defined in the bridge seem not to be activated.

@blegat
Copy link
Member

blegat commented Nov 14, 2019

I guess the fallback assumes 0?

Yes :)

@matbesancon matbesancon reopened this Nov 18, 2019
if attr.N == 1
return MOI.get(model, MOI.VariablePrimal(), bridge.z_variable_index)
end
wv = MOI.get(model, MOI.VariablePrimal(), bridge.w_variable_index)
Copy link
Member

Choose a reason for hiding this comment

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

This should be MOI.VariablePrimal(attr.N)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how come? w is a scalar variable

Copy link
Member

Choose a reason for hiding this comment

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

For the Nth result, you have VariablePrimal(N), ConstraintPrimal(N) and ConstraintDual(N). The value of ConstraintPrimal(N) is the value of the function when the variables have values VariablePrimal(N)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, that's not really intuitive, I thought ConstraintPrimal(N) would be the primal of the N-th function in the vector function. I'll update that

@matbesancon
Copy link
Contributor Author

@blegat after thoughts, not sure MOI.ConstraintPrimal should be implemented. You cannot test it without a value set, and you cannot support setting a ConstraintPrimal without other things (see error below)

Getting primal attributes: Error During Test at /home/mbesancon/.julia/dev/MathOptInterface/test/Bridges/Constraint/indicator_sos.jl:179
  Got exception outside of a @test
  ArgumentError: `supports` is not defined for MathOptInterface.ConstraintPrimal(1), it is only defined for attributes such that `is_copyable` returns `true`.
  Stacktrace:
   [1] supports(::MathOptInterface.Utilities.Model{Float64}, ::MathOptInterface.ConstraintPrimal, ::Type{MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64},MathOptInterface.LessThan{Float64}}}) at /home/mbesancon/.julia/dev/MathOptInterface/src/attributes.jl:197
   [2] set(::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}, ::MathOptInterface.ConstraintPrimal, ::MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64},MathOptInterface.LessThan{Float64}}, ::Float64) at /home/mbesancon/.julia/dev/MathOptInterface/src/Utilities/universalfallback.jl:356

@blegat
Copy link
Member

blegat commented Nov 25, 2019

If you set the variable values, the mock optimizer uses them to compute ConstraintPrimal using get_fallback. I dont't why this bridge should not support it.

@matbesancon
Copy link
Contributor Author

Alright ConstraintPrimal re-implemented and passing

@blegat blegat added this to the v0.9.8 milestone Nov 26, 2019
@blegat blegat merged commit 8618ca0 into jump-dev:master Nov 26, 2019
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.

4 participants