Add result fallbacks #447

blegat · 2018-08-03T19:13:32Z

As discussed in #442 and JuliaOpt/LinQuadOptInterface.jl#41, some solver API do not contain any getters for result informations that can be deduced for variable primals and constraint duals of non-variablewise constraints.

These include the following:

ObjectiveValue: can be deduced from VariablePrimal and ObjectiveFunction
ObjectiveBound: can be deduced from ConstraintDual and constants in ConstraintFunction's and ConstraintSet's
ConstraintPrimal: can be deduced from ConstraintFunction's and VariablePrimal's
ConstraintDual of variablewise constraints: can be deduced from ConstraintDual of non-variablewise constraints, ObjectiveFunction and ConstraintFunction's.

This PR

moves the fallback already used by MockOptimizer for ObjectiveValue and ConstraintPrimal to a new get_fallback function that is independent of the MockOptimizer.
creates a fallback for ConstraintDual and use it for MockOptimizer in the tests (it can also be useful for solving Farkas duals for variable bounds JuliaOpt/LinQuadOptInterface.jl#41)
Introduce set_dot and dot_coefficients as new set functions that need to be implemented. They have a default implementation for non-triangle sets.
renames evalobjective to eval_objective_value in MockOptimizer to make place to eval_objective_bound

TODO:

Document get_fallback

mlubin · 2018-08-04T13:14:57Z

src/sets.jl

@@ -306,6 +365,29 @@ end

 dimension(s::PositiveSemidefiniteConeSquare) = s.side_dimension^2

+function square_dot(x::Vector{T}, y::Vector{T}, dim::Int, offset::Int) where T
+    @show x


Debugging code?

mlubin · 2018-08-04T13:15:14Z

src/sets.jl

 dimension(s::Union{LogDetConeSquare, RootDetConeSquare}) = 1 + s.side_dimension^2
+#function set_dot(x::Vector, y::Vector, set::Union{LogDetConeSquare,


Commented code?

mlubin · 2018-08-04T13:16:51Z

src/Utilities/results.jl

@@ -0,0 +1,196 @@
+"""


Explain the purpose of this file and how solver wrappers should use it.

mlubin · 2018-08-04T13:20:37Z

src/sets.jl

+Return the scalar product between a vector `x` of the set `set` and a vector
+`y` of the dual of the set `s`.
+"""
+function set_dot(x::Vector, y::Vector, set::AbstractVectorSet)


I think these methods belong in MOIU rather than MOI. They're helpers for implementing MOI and aren't necessary for the API.

If someone defines a new set with a non-trivial inner product, he is forced to define set_dot and dot_coefficients otherwise all codes using these functions fails. As this is mandatory, this is part of the API, isn't it ?

Is there code using these functions outside of MOIU? The get_fallback should be opt-in by the solver and not automatic.

codecov-io · 2018-08-06T17:19:21Z

Codecov Report

Merging #447 into master will decrease coverage by 0.02%.
The diff coverage is 92.92%.

@@            Coverage Diff             @@
##           master     #447      +/-   ##
==========================================
- Coverage   95.28%   95.26%   -0.03%     
==========================================
  Files          42       43       +1     
  Lines        4983     5087     +104     
==========================================
+ Hits         4748     4846      +98     
- Misses        235      241       +6

Impacted Files	Coverage Δ
src/Utilities/mockoptimizer.jl	`95.7% <100%> (+0.49%)`	⬆️
src/sets.jl	`100% <100%> (ø)`	⬆️
src/Test/contconic.jl	`100% <100%> (ø)`	⬆️
src/attributes.jl	`96.61% <100%> (+0.05%)`	⬆️
src/Utilities/model.jl	`97.1% <50%> (-0.4%)`	⬇️
src/Utilities/results.jl	`92.94% <92.94%> (ø)`

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 c7c4d2d...62285b9. Read the comment docs.

blegat · 2018-08-08T16:33:15Z

Good to merge ?

mlubin

Sorry for the delay, busy week.

mlubin · 2018-08-09T02:34:34Z

src/Utilities/results.jl

@@ -0,0 +1,283 @@
+# This file contains the implementation of different methods for the
+# `get_fallback` function. These methods can be used by solver wrappers as
+# fallbacks for implemented the `get` method when the solver API does not


"for implemented" -> "for implementing"

mlubin · 2018-08-09T02:35:48Z

src/Utilities/results.jl

+            func = MOI.get(model, MOI.ConstraintFunction(), constraint_index)
+            if (F == MOI.SingleVariable && func.variable == vi) ||
+               (F == MOI.VectorOfVariables && vi in func.variables)
+               error("Fallback getter for variable constraint dual does not support other variable-wise constraints on the variable.")


Nit: line break the error message (using * to concat strings)

mlubin · 2018-08-09T02:39:07Z

src/Utilities/results.jl

+                       ::Type{<:Union{MOI.ScalarQuadraticFunction,
+                                      MOI.VectorQuadraticFunction}},
+                       ::Type{<:MOI.AbstractSet})
+    error("Fallback getter for variable constraint dual only supports affine constraint functions.")


What happens if a user sees this error message? Does it indicate an internal error in the solver wrapper or that the user did something wrong? (Same with similar errors below.)

An error on the wrapper, the wrapper shouldn't call this if there are double variable bounds or quadratic functions

This should be reflected in the error messages to help users if they see it.

mlubin · 2018-08-09T02:40:25Z

src/Utilities/results.jl

+    return 0.0
+end
+
+function variable_dual(::MOI.ModelLike,


This function has lots of methods that aren't obvious to understand. It needs a docstring or at least a comment.

mlubin · 2018-08-09T02:47:26Z

src/attributes.jl

@@ -380,6 +380,24 @@ it has non-integer coefficient and `F` is `ScalarAffineFunction{Int}`.
 """
 struct ObjectiveFunction{F<:AbstractScalarFunction} <: AbstractModelAttribute end

+"""
+    ObjectiveFunctionType()


Add this to the list of attributes in the docs.

mlubin

Good to merge after tweaking the error message as discussed inline.

mlubin reviewed Aug 4, 2018

View reviewed changes

blegat force-pushed the bl/resultfallback branch from 8e470e1 to 6daeee3 Compare August 6, 2018 16:49

blegat force-pushed the bl/resultfallback branch from 6daeee3 to 64b7db4 Compare August 8, 2018 16:21

blegat added 8 commits August 8, 2018 18:23

Add result fallbacks

1bf8896

evalobjective -> eval_objective_value

77d00d6

Document get_fallback

6cfdb45

Remove square_dot

4b589d5

Explain purpose of result file

6fbd995

Move set_dot and dot_coefficients to MOIU

3f83af3

Missing <:

e541f6d

Fix mock tests

64b7db4

mlubin reviewed Aug 9, 2018

View reviewed changes

Address comments

225a8e0

mlubin approved these changes Aug 10, 2018

View reviewed changes

Clarify error messages

62285b9

blegat merged commit 8c06b3c into master Aug 10, 2018

blegat deleted the bl/resultfallback branch August 28, 2018 14:32

blegat mentioned this pull request Sep 23, 2018

Accessor for ObjectiveFunction type #238

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add result fallbacks #447

Add result fallbacks #447

blegat commented Aug 3, 2018 •

edited

Loading

mlubin Aug 4, 2018

mlubin Aug 4, 2018

mlubin Aug 4, 2018

mlubin Aug 4, 2018

blegat Aug 4, 2018

mlubin Aug 4, 2018

codecov-io commented Aug 6, 2018 •

edited

Loading

blegat commented Aug 8, 2018

mlubin left a comment

mlubin Aug 9, 2018

mlubin Aug 9, 2018

mlubin Aug 9, 2018

blegat Aug 9, 2018

mlubin Aug 10, 2018 •

edited

Loading

mlubin Aug 9, 2018

mlubin Aug 9, 2018

mlubin left a comment

		dimension(s::Union{LogDetConeSquare, RootDetConeSquare}) = 1 + s.side_dimension^2
		#function set_dot(x::Vector, y::Vector, set::Union{LogDetConeSquare,

Add result fallbacks #447

Add result fallbacks #447

Conversation

blegat commented Aug 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Aug 6, 2018 • edited Loading

Codecov Report

blegat commented Aug 8, 2018

mlubin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlubin Aug 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlubin left a comment

Choose a reason for hiding this comment

blegat commented Aug 3, 2018 •

edited

Loading

codecov-io commented Aug 6, 2018 •

edited

Loading

mlubin Aug 10, 2018 •

edited

Loading