Indicator constraint support #709

matbesancon · 2019-04-11T20:15:14Z

First draft for the addition of indicator constraints to MOI. There is evidence for multiple solvers handling them:
Gurobi
CPLEX
SCIP

One thing we would want to add (suggestion from @leethargo) is a bridge for solvers supporting SOS1 but not indicator constraints natively. From 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.

src/sets.jl

codecov-io · 2019-04-11T21:08:46Z

Codecov Report

Merging #709 into master will increase coverage by 0.09%.
The diff coverage is 99.04%.

@@            Coverage Diff             @@
##           master     #709      +/-   ##
==========================================
+ Coverage   93.73%   93.83%   +0.09%     
==========================================
  Files          54       54              
  Lines        5572     5677     +105     
==========================================
+ Hits         5223     5327     +104     
- Misses        349      350       +1

Impacted Files	Coverage Δ
src/Test/intlinear.jl	`100% <100%> (ø)`	⬆️
src/sets.jl	`93.75% <66.66%> (-1.81%)`	⬇️

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 9970dbd...caeb4c4. Read the comment docs.

src/sets.jl

blegat · 2019-04-12T10:50:44Z

src/sets.jl

@@ -449,7 +466,7 @@ function Base.copy(set::Union{Reals, Zeros, Nonnegatives, Nonpositives,
                              PositiveSemidefiniteConeSquare,
                              LogDetConeTriangle, LogDetConeSquare,
                              RootDetConeTriangle, RootDetConeSquare,
-                              Integer, ZeroOne, Semicontinuous, Semiinteger})
+                              Integer, ZeroOne, Semicontinuous, Semiinteger, IndicatorSet})


It is not an isbits type since it contains a vector. The copy should probably copy this vector too

You're right, I'll re-implement copy. Should we make the vector type generic?

src/sets.jl

rschwarz · 2019-04-12T11:11:41Z

What about having a set parameter for whether y should be 0 or 1 to activate the constraint?

Then the disjunction ax ≤ b v cx ≤ d could be expressed by the two constraints

(y, x) in IndicatorSet(1, a, LessThan(b))
(y, x) in IndicatorSet(0, c, LessThan(d))

reusing the y variable. Otherwise, another z would have to be introduced with y + z == 1?

matbesancon · 2019-04-12T11:45:36Z

Should we make the indicator set generic on vector type?
IndicatorSet{T,S <: ScalarSet,V <: AbstractVector{<:Real}}

mlubin · 2019-04-12T11:55:47Z

What are the implications of having non isbits sets? How does this interact with MOF?

One way to avoid storing the a vector in the set would be to store it in the function, e.g.,

Set of (y, x) that satisfy the indicator constraint:
    y ∈ 𝔹, y = 1 ==> x ∈ S

Then use VAF to write down the constraints, e.g.,

(1.0 * y, 2*x + 3*z) in IndicatorSet(LessThan(b))

It's an error if the first component of the VAF isn't 1.0 times some variable.

odow · 2019-04-12T12:04:38Z

The SOS sets already have weight vectors, so this isn't breaking new ground. No problem on the MOF front

matbesancon · 2019-04-12T12:16:49Z

see last version for generic vector

odow · 2019-04-12T12:20:54Z

Oh, wait. I misread miles' concern. Not sure how abstractvector would play with MOF. I'd prefer the vectoraffinefunction in set approach.

This reverts commit 6e57763.

matbesancon · 2019-04-12T12:27:48Z

problem with VectorAffine in Indicator is that it requires Set to hold variables. Also keeping a vector of a is consistent with SOS

mlubin · 2019-04-12T12:38:38Z

problem with VectorAffine in Indicator is that it requires Set to hold variable

What do you mean by this?

matbesancon · 2019-04-12T12:40:27Z

Either VectorAffine has a weird special first term (1.0, y) or the y needs to be specified in the set

mlubin · 2019-04-12T12:42:29Z

The weird special term is fine with me. That's what we do for cones.

We have machinery already to translate JuMP AffExprs into VAF, so today you could write:

@constraint(model, [y, 2x + 3z] in IndicatorSet(LessThan(b)))

OTOH,

@constraint(model, [y, x, z] in IndicatorSet([2.0, 3.0], LessThan(b)))

seems like a regression in modeling because you have to manually create a list of coefficients.

matbesancon · 2019-04-12T12:44:40Z

Ok I see. [y, 2x + 3z] would be specially parsed at the JuMP level? (Meaning, different from [y, 2x, 3z] or [y + 2x + 3z]

mlubin · 2019-04-12T12:47:13Z

[y, 2x + 3z] would be specially parsed at the JuMP level? (Meaning, different from [y, 2x, 3z] or [y + 2x + 3z]

Yes, those are all parsed differently.

matbesancon · 2019-04-12T12:57:26Z

one thing is that the set cannot determine the dimension of the expected function

mlubin · 2019-04-12T13:00:33Z

The dimension of the set I'm proposing (under the MOI definition) would always be 2.

matbesancon · 2019-04-12T13:01:59Z

ok I hadn't thought of counting like this, so the whole affine term a^T x count as dim 1 right?

odow · 2019-04-12T13:16:30Z

What about

@constraint(model, 2x1 + x2 in MOI.IndicatorSet(y=>1, MOI.GreaterThan(1.0)))

So

struct IndicatorSet{S}
    variable::Pair{MOI.VariableIndex, Bool}
    set::S
end

Then if S is a scalar set, it works for any scalar function, and if S is a vector set, it works for any vector function.

matbesancon · 2019-04-12T13:19:34Z

Ok, one design consideration I tried to keep is to represent the constraint as closely as possible to what solvers take as input

matbesancon · 2019-04-23T21:30:51Z

Fixed most of the comments. @blegat I didn't find anything for testing new sets with MockOptimizer.
SOS constraints are not tested using it (just basic unit tests in /test/sets.jl

src/Test/intlinear.jl

test/Test/intlinear.jl

matbesancon · 2019-04-24T15:57:19Z

tests passing cc @blegat @mlubin

src/sets.jl

blegat

Almost there. Missing a (tested) copy implementation that copies the internal set

matbesancon · 2019-04-24T21:42:52Z

Thanks, added in last commit

blegat · 2019-04-24T22:10:29Z

test/sets.jl

+            s1_copy = copy(s1)
+            s2_copy = copy(s2)
+            @test s1_copy isa MOI.IndicatorSet{MOI.ACTIVATE_ON_ONE}
+            @test s1 == s1_copy


Check that modifying the set in s1 does not modify the set in s1_copy. You will need a scalar set which is mutable

do we even have one? I understand the importance of testing it does not happen, but we are calling copy on the subtest, so if error there is, it's because copy is not properly implemented on it

@matbesancon LessThan{BigInt} is mutable because BigInt is mutable. You call also create a dummy mutable coefficient type or a dummy mutable set just for the test.

we are calling copy on the subtest

That's what we want to test. If you remove this copy on the subset, the tests still pass while they shouldn't

@blegat I played a bit with this idea. Fundamentally, this is breaking an assumption about numbers that will be everywhere in MOI, this PR doesn't change that. BigInt being mutable is just a constraint for their construction as far as I know, not supposed to be used.

The implementation from src/sets:

# isbits types, nothing to copy function Base.copy(set::Union{Reals, Zeros, Nonnegatives, Nonpositives, GreaterThan, LessThan, EqualTo, Interval, SecondOrderCone, RotatedSecondOrderCone, GeometricMeanCone, ExponentialCone, DualExponentialCone, PowerCone, DualPowerCone, PositiveSemidefiniteConeTriangle, PositiveSemidefiniteConeSquare, LogDetConeTriangle, LogDetConeSquare, RootDetConeTriangle, RootDetConeSquare, Integer, ZeroOne, Semicontinuous, Semiinteger}) return set end

Is then false because it should check isbits on the set

Good point. We could defer the question of whether we should copy the field of EqualTo... to a separate issue.
Still you might be able to create a mutable abstract set with Float64 to test the fact we ce copy the set for IndicatorSet

@blegat see issue #721, and tests added here for IndicatorSet copy

mlubin

Looking good. Just a few minor comments.

src/Test/intlinear.jl

test/runtests.jl

src/Test/intlinear.jl

first draft indicator cons

049e4cb

odow reviewed Apr 11, 2019

View reviewed changes

src/sets.jl Show resolved Hide resolved

odow reviewed Apr 11, 2019

View reviewed changes

src/sets.jl Outdated Show resolved Hide resolved

matbesancon added 3 commits April 11, 2019 22:43

fixed docstring

a550983

fixed LessThan

d17ba8e

fix import

3bce12e

blegat reviewed Apr 12, 2019

View reviewed changes

src/sets.jl Outdated Show resolved Hide resolved

last SmallerThan

2ca54d9

blegat reviewed Apr 12, 2019

View reviewed changes

copy correct impl

c3c2fa6

generic vec

6e57763

Revert "generic vec"

9db213a

This reverts commit 6e57763.

matbesancon added 2 commits April 23, 2019 23:21

fix test 2

24af034

added test

f60c3c8

blegat reviewed Apr 24, 2019

View reviewed changes

src/Test/intlinear.jl Outdated Show resolved Hide resolved

remove unsupported tests, names

1560400

blegat reviewed Apr 24, 2019

View reviewed changes

test/Test/intlinear.jl Outdated Show resolved Hide resolved

matbesancon added 2 commits April 24, 2019 13:44

test passing

74836af

fix support

e89d734

blegat requested changes Apr 24, 2019

View reviewed changes

src/sets.jl Outdated Show resolved Hide resolved

src/sets.jl Outdated Show resolved Hide resolved

fix style

d9565d6

blegat self-requested a review April 24, 2019 20:43

blegat requested changes Apr 24, 2019

View reviewed changes

copy implementation and test

3c0155e

blegat reviewed Apr 24, 2019

View reviewed changes

mlubin reviewed Apr 25, 2019

View reviewed changes

src/Test/intlinear.jl Outdated Show resolved Hide resolved

src/Test/intlinear.jl Outdated Show resolved Hide resolved

test/runtests.jl Outdated Show resolved Hide resolved

src/Test/intlinear.jl Show resolved Hide resolved

matbesancon added 3 commits April 25, 2019 12:17

fix comments, added test for ACTIVE_ON_ZERO

c7f926e

test support indicator

9d85fee

explicit comment

82d3d56

mlubin approved these changes Apr 27, 2019

View reviewed changes

matbesancon added 2 commits April 28, 2019 18:32

fix conflict

01f8690

add mutable test

caeb4c4

matbesancon mentioned this pull request Apr 29, 2019

Base.copy sets is not checked for isbits type #721

Closed

blegat approved these changes Apr 29, 2019

View reviewed changes

blegat merged commit c1d8f6a into jump-dev:master Apr 30, 2019

matbesancon deleted the indicator-cons branch April 30, 2019 11:20

odow mentioned this pull request Jun 18, 2019

Add IndicatorSet jump-dev/MathOptFormat#7

Merged

blegat mentioned this pull request Oct 4, 2019

Complementarity constraint #771

Closed

odow mentioned this pull request May 20, 2020

[WIP] Evaluation distance to set #1023

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Indicator constraint support #709

Indicator constraint support #709

matbesancon commented Apr 11, 2019

codecov-io commented Apr 11, 2019 •

edited

Loading

blegat Apr 12, 2019

matbesancon Apr 12, 2019

rschwarz commented Apr 12, 2019

matbesancon commented Apr 12, 2019

mlubin commented Apr 12, 2019 •

edited

Loading

odow commented Apr 12, 2019

matbesancon commented Apr 12, 2019

odow commented Apr 12, 2019

matbesancon commented Apr 12, 2019

mlubin commented Apr 12, 2019 •

edited

Loading

matbesancon commented Apr 12, 2019

mlubin commented Apr 12, 2019 •

edited

Loading

matbesancon commented Apr 12, 2019

mlubin commented Apr 12, 2019

matbesancon commented Apr 12, 2019

mlubin commented Apr 12, 2019

matbesancon commented Apr 12, 2019

odow commented Apr 12, 2019

matbesancon commented Apr 12, 2019

matbesancon commented Apr 23, 2019

matbesancon commented Apr 24, 2019

blegat left a comment

matbesancon commented Apr 24, 2019

blegat Apr 24, 2019

matbesancon Apr 25, 2019

blegat Apr 27, 2019 •

edited

Loading

matbesancon Apr 28, 2019

blegat Apr 28, 2019

matbesancon Apr 29, 2019

mlubin left a comment

Indicator constraint support #709

Indicator constraint support #709

Conversation

matbesancon commented Apr 11, 2019

codecov-io commented Apr 11, 2019 • edited Loading

Codecov Report

blegat Apr 12, 2019

Choose a reason for hiding this comment

matbesancon Apr 12, 2019

Choose a reason for hiding this comment

rschwarz commented Apr 12, 2019

matbesancon commented Apr 12, 2019

mlubin commented Apr 12, 2019 • edited Loading

odow commented Apr 12, 2019

matbesancon commented Apr 12, 2019

odow commented Apr 12, 2019

matbesancon commented Apr 12, 2019

mlubin commented Apr 12, 2019 • edited Loading

matbesancon commented Apr 12, 2019

mlubin commented Apr 12, 2019 • edited Loading

matbesancon commented Apr 12, 2019

mlubin commented Apr 12, 2019

matbesancon commented Apr 12, 2019

mlubin commented Apr 12, 2019

matbesancon commented Apr 12, 2019

odow commented Apr 12, 2019

matbesancon commented Apr 12, 2019

matbesancon commented Apr 23, 2019

matbesancon commented Apr 24, 2019

blegat left a comment

Choose a reason for hiding this comment

matbesancon commented Apr 24, 2019

blegat Apr 24, 2019

Choose a reason for hiding this comment

matbesancon Apr 25, 2019

Choose a reason for hiding this comment

blegat Apr 27, 2019 • edited Loading

Choose a reason for hiding this comment

matbesancon Apr 28, 2019

Choose a reason for hiding this comment

blegat Apr 28, 2019

Choose a reason for hiding this comment

matbesancon Apr 29, 2019

Choose a reason for hiding this comment

mlubin left a comment

Choose a reason for hiding this comment

codecov-io commented Apr 11, 2019 •

edited

Loading

mlubin commented Apr 12, 2019 •

edited

Loading

mlubin commented Apr 12, 2019 •

edited

Loading

mlubin commented Apr 12, 2019 •

edited

Loading

blegat Apr 27, 2019 •

edited

Loading