Skip to content

Conversation

@odow
Copy link
Member

@odow odow commented Oct 8, 2019

This PR adds the set Complements(dimension) which defines a mixed complementarity constraint.

The main targets are

  • KNITRO.jl, which will implement VectorOfVariables-in-Complements (cc @frapac)
  • PATH.jl, which will implement VectorQuadraticFunction-in-Complements and VectorAffineFunction-in-Complements, implementing both allows us to short-circuit the Jacobian evaluation the affine case. VectorOfVariables-in-Complements will be handled by bridges.

Closes #771

I've left more obscure complementarity constraints, like conic constrained x compared to lb <= x <= ub, un-mentioned. @frapac does KNITRO just look at the bounds? Or does it do something different?

cc @xhub

@codecov-io
Copy link

codecov-io commented Oct 8, 2019

Codecov Report

Merging #913 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #913      +/-   ##
==========================================
+ Coverage    95.2%   95.27%   +0.07%     
==========================================
  Files          82       83       +1     
  Lines        8751     9059     +308     
==========================================
+ Hits         8331     8631     +300     
- Misses        420      428       +8
Impacted Files Coverage Δ
src/Test/UnitTests/basic_constraint_tests.jl 98% <ø> (ø) ⬆️
src/Utilities/model.jl 94.63% <ø> (ø) ⬆️
src/Test/nlp.jl 20.22% <100%> (+20.22%) ⬆️
src/sets.jl 95.06% <100%> (+0.06%) ⬆️
src/attributes.jl 86.32% <0%> (-0.84%) ⬇️
src/Test/UnitTests/solve.jl 100% <0%> (ø) ⬆️
src/Bridges/Bridges.jl 100% <0%> (ø) ⬆️
src/Bridges/debug.jl 100% <0%> (ø)
src/Bridges/lazy_bridge_optimizer.jl 93.85% <0%> (+0.6%) ⬆️
src/Utilities/mockoptimizer.jl 92.64% <0%> (+2.1%) ⬆️

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 e29f1ce...ad1d56b. Read the comment docs.

Copy link
Contributor

@frapac frapac left a comment

Choose a reason for hiding this comment

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

Thanks @odow for adding this in MOI!

@blegat
Copy link
Member

blegat commented Oct 8, 2019

If at some point we define VariationalInequality(C) that means as @xhub suggested then Complements(n) will simply be VariationalInequality(Nonnegatives(n)).
What should we do then ?

  1. Define const Complements = VariationalInequality{Nonnegatives}
  2. Define a bridge mapping VariationalInequality{Nonnegatives} to Complements and vice versa

In MOI, we don't currently have a set for which the type is a special case of the type of another one so I think 1. would make more sense.
If we go with 1., we should make sure we don't get any backward compatibility issue by transforming Complements from a struct to a const.

@odow
Copy link
Member Author

odow commented Oct 8, 2019

Note that our definition of complements isn't VariationalInequality{Nonnegatives}, it's really VariationalInequality(Hypercube(lb, ub)) so I think a bridge is the way to go.

In MOI, we don't currently have a set for which the type is a special case of the type of another one

Sure we do: Integer and ZeroOne, GreaterThan and Interval.

@odow
Copy link
Member Author

odow commented Oct 8, 2019

Really, there is nothing stopping us from interpreting the current Complements as VariationalInequality.

The current definition is just a relation between F(x) and x with the bounds added separately. So the cone C can just be inferred from the constraints on x. Maybe we should change the docstring to reflect this.

@blegat
Copy link
Member

blegat commented Oct 8, 2019

The current definition is just a relation between F(x) and x with the bounds added separately.

Bridging currently assume that we can treat each constraint independently so it will be easy to bridge VariationalInequality to Complements but not Complements to VariationalInequality as you would need a mechanism for the bridge to say to the bridge optimizer to notify him when the bounds on the variable changes (we would also need something similar for the QuadtoSOC bridge to support x^2 + y^2 <= t^2 as we need to be sure that there is a bound on t so maybe we could add that at some point.

I prefer VariationalInequality(Hyperrectangle(lb, ub)) over Complements as all the information is inside the constraint and it does not depend on other constraint.
However, if Complements is a close match to Knitro and PATH interface (EDIT I have checked your PATH interface, Complements is indeed an exact match) then it makes sense to have both. It's kind of similar to Quad vs SOC. SOC (resp. VI) are preferred because they are easy to bridge into Quad (resp. Complements) and the other side depends on variable bounds.

@odow
Copy link
Member Author

odow commented Oct 8, 2019

Bridging currently assume that we can treat each constraint independently so it will be easy to bridge VariationalInequality to Complements but not Complements to VariationalInequality

Sure. I envisaged a one-way VariationalInequality to Complements bridge.

Since we have two solvers that currently support Complements as written, we could keep Complements for now. If, in the future, we have multiple solvers that support more general variational inequalities (e.g., PATH-VI), then we can revisit this discussion, probably after those solvers have experimented with the syntax as external sets.

@blegat
Copy link
Member

blegat commented Oct 8, 2019

We also need to add it to docs/src/apireference.md

@odow
Copy link
Member Author

odow commented Oct 8, 2019

Before merging, we should wait for @xhub to sign-off.

@blegat blegat added this to the v0.9.5 milestone Oct 8, 2019
@frapac
Copy link
Contributor

frapac commented Oct 9, 2019

@frapac does KNITRO just look at the bounds? Or does it do something different?

Once complementarity constraints defined inside Knitro, the solver forces the lower-bounds of the variables to be equal to 0.

@blegat
Copy link
Member

blegat commented Oct 9, 2019

Once complementarity constraints defined inside Knitro, the solver forces the lower-bounds of the variables to be equal to 0.

So Knitro.jl will throw an error of the variable lower are nonzero and if the upper bound is not +infinity ?

@frapac
Copy link
Contributor

frapac commented Oct 9, 2019

If the variable LB is lower than 0, no error is returned (Knitro should set automatically the lower bound equal to 0, but it appears that the control checks have been updated). If the variable LB is greater than 0, then Knitro returns:

ERROR: Infeasible problem specification.
       Variables x[2] and x[5] are complementary but
       constrained to be strictly positive.  At least one
       of these variables must be 0 at the solution.

odow and others added 2 commits October 10, 2019 15:51
* Test: add QP complementarity test

* Add Mock test for QP complementarity test

* Fix with comments in PR#918
Copy link

@xhub xhub left a comment

Choose a reason for hiding this comment

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

Sorry for the late review. That looks very good. I just had a few suggestions about the wording in the doc.

@xhub
Copy link

xhub commented Oct 14, 2019

I am not sure I followed the discussion of Complements vs VariationalInequality.
As @odow said, nothing prevents us from encoding a VI using Complements.
I understand that @blegat propose that we have a VariationalInequality set where the data for the set C is stored inside. That could be a great way to store the data.
It could be used whenever C can be described as Ax + b in K and K is a cone.

One just has to see how to educate the user about the need to specify the set C in this way and not via @constraints macros.

@blegat
Copy link
Member

blegat commented Oct 15, 2019

One just has to see how to educate the user about the need to specify the set C in this way and not via @constraints macros.

What do you mean ? The user can do @constraint(model, [A*x + b; x] in MOI.VariationalInequality(C)) to model where F(x) = Ax + b.

@blegat blegat added this to the v0.9.6 milestone Oct 15, 2019
@odow
Copy link
Member Author

odow commented Oct 17, 2019

As I've said before, I think we can leave the VariationalInequality set for a future solver. But I think @blegat's last suggestion is the way to go.

@xhub Are the latest changes okay?

@xhub
Copy link

xhub commented Oct 21, 2019

@odow Thanks for making the changes. I am really happy with Complements as just some tagging information. The only part that I would maybe think about changing is the documentation for Complements. With the current implementation, and proper support from a solver, nothing prevents me from specifying a general polyhedral VI. Currently the doc mentions MCP, which is good. If we want to change the documentation, we could do that later.

@blegat I am not sure what V is in your example. Maybe it is best to move on on the Complements.
For VariationalInequalities I was thinking we could have the following

struct VariationalInequalities
   A::SparseMatrix
   b::AbstractVector
   K::Union{MOI.Zeros, MOI.Nonnegatives, MOI.Nonpositives}
end

I was thinking of a case where the user does the following in a variational inequality problem definition

@constraint(model, [A*x + b; x] in MOI.VariationalInequalities(H, k, MOI.Nonnegatives))
@constraint(model, x[2] >= 5)

Then, the full definition of C is the intersection of Hx <= k and x[2] >= 5. Then, the burden of
I must say that I may be too pessimistic about the user's knowledge. I may have in mind people who are not optimizers by training, rather use optimization algorithm as a tool.

@blegat
Copy link
Member

blegat commented Oct 21, 2019

I am not sure what V is in your example.

It should be C, I have corrected it. About the A and b field in VariationalInequality, I would prefer keeping VariationalInequality simple and have a new set AffineTransformation and so that the user does VariationalInequality(AffineTranformation(A, b, MOI.Nonnegatives(2))) but that's out the scope of this PR.

@odow
Copy link
Member Author

odow commented Oct 21, 2019

If we want to change the documentation, we could do that later.

Yeah, it's easier to change to be more general later on.

@odow odow merged commit 97a6483 into master Oct 21, 2019
@odow odow deleted the od/complements branch October 21, 2019 15:01
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.

Complementarity constraint

6 participants