Remove error from feasibility sense

[guilhermebodin]

close #155

@blegat This makes it work but I am not sure what is the formal dual of a feasibility problem.

If we assume feasibility is Min 0 should the dual be the same as if we assume Max 0 or maybe it's whatever. Also, the dual of the dual would not be a feasibility-sense problem.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.74%. Comparing base (14edeae) to head (c333945).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #162   +/-   ##
=======================================
  Coverage   94.73%   94.74%           
=======================================
  Files          13       13           
  Lines         703      704    +1     
=======================================
+ Hits          666      667    +1     
  Misses         37       37           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[odow]

If we assume feasibility is Min 0 should the dual be the same as if we assume Max 0 or maybe it's whatever. Also, the dual of the dual would not be a feasibility-sense problem.

Devils advocate: does a feasibility sense problem have a dual? It doesn't have an objective function.

[joaquimg]

I would say there is no dual.
If someone wants a dual, select a sense. Coefficients may remain zero in the objective.

[guilhermebodin]

I agree @odow. The other sane option would be to make a more informative error message such as:

error(primal_sense, " does not have a well-defined dual problem. A solution for this problem is to add the objective `Min 0` and assume that it is a minimization problem.")

[odow]

Yeah a more informative error sounds good.

[blegat]

If a user solve a conic problem, he should always try with and without Dualization because it can change lot (it also impacts the low-rank-ness of the solution which is important for instance for Burer-Monteiro-like and ProxSDP). By the way, we should insist more about that in the JuMP doc. I'm going to do it in the SumOfSquares block but this issue is blocking it.
Having Dualization fail for FEASIBILITY_SENSE is a big blocker.
In MOI, the dual for max sense is defined as the dual of min sense where the objective is reversed so there should be no difference in which sense we choose here (but I would choose MIN_SENSE)

[odow]

Having Dualization fail for FEASIBILITY_SENSE is a big blocker.

Why? The user could just set a min 0 objective. SumOfSquares could do so too.

[blegat]

Why? The user could just set a min 0 objective. SumOfSquares could do so too.

That wouldn't really be an option because SumOfSquares just defines new MOI sets and MOI bridges.

The dual of MAX_SENSE is really just defined in MOI as the dual of MIN_SENSE with the reversed objective except that the dual is with a MIN_SENSE and not a MAX_SENSE
Here, because the objective is zero, reversing it does not change anything so the only thing that would be different is that the dual would be a minimum or a maximum. I think defaulting on min 0 and having a max in the dual is a safe choice.

Another option is to have a keyword in dualize so that if you use dualize without a sense and without setting that keyword you get an error.
Then Dualization.Optimizer would set that keyword to default to min because the users won't see the difference because the dualization is transparant in the solution process.

[odow]

What was the upstream bug? https://github.com/jump-dev/SumOfSquares.jl/pull/283/files

You tried to do

import Dualization, SCS
model = SOSModel(() -> Dualization.Optimizer(SCS.Optimizer))
@constraint(model, motzkin >= 0)
optimize!(model)

and it didn't work because no objective was set? Then either SOSModel could always start by setting min 0 as an objective, or you could say to the user "hey, set an objective."

It currently doesn't work, so it isn't breaking to change an error message.

[blegat]

jump-dev/SumOfSquares.jl#283 is not about a breaking change in Dualization. The tutorial started failing because of the bug that was fixed in jump-dev/CSDP.jl#89 so I tried SCS but I couldn't because dualization does not support feasibility sense so I went back to using CSDP since I fixed the bug in CSDP.

I understand that choosing this objective sense in dualize is confusing since it affects the objective sense of the dual which is in the dualized problem.
However, for Optimizer, it really shouldn't be a problem. It's goal is to transparently solve the dual problem in the solver. The objective sense given in the solver really shouldn't matter. If you don't want to add a keyword to the dualize function, then Optimizer can also add a filter layer that will change the getters of ObjectiveSense and ObjectiveFunction like in https://github.com/ericphanson/SDPAFamily.jl/pull/66/files#diff-33ec7fe2aadeaed51062fede6400fae5468eda2717c631927e70a184e3b27ccd

and it didn't work because no objective was set? Then either SOSModel could always start by setting min 0 as an objective, or you could say to the user "hey, set an objective."

SOSModel is only needed if you want to use the >= syntax otherwise you can just use Model. Now that we have @constraint(model, .. >= ..., PSDCone()) I could actually get rid of it in favor of @constraint(model, ... >= ..., SOSCone()) so Model should be enough. So I don't have any opportunity to add an objective.
Also, for the same reason, it's weird to add an objective in Dualization, it is even weirder in SumOfSquares.
SumOfSquares has nothing to do with this. Dualization needs an objective to be set and Dualization is important for conic problems and SumOfSquares creates conic problems but doing this in SumOfSquares looks like a weird hack.