Refactor objective sense (#1526)

* Refactor objective sense

Use MOI.OptimizationSense instead of symbols at the JuMP level.
Add `set_objective_sense`.
Closes #1459

* Comments

src/JuMP.jl

@@ -327,20 +327,21 @@ Return the objective value after a call to `optimize!model)`.
 objective_value(model::Model) = MOI.get(model, MOI.ObjectiveValue())
 
 """
-    objective_sense(model::Model)
+    objective_sense(model::Model)::MathOptInterface.OptimizationSense
 
-Return the objective sense, `:Min`, `:Max`, or `:Feasibility`.
+Return the objective sense.
 """
 function objective_sense(model::Model)
-    moisense = MOI.get(model, MOI.ObjectiveSense())
-    if moisense == MOI.MinSense
-        return :Min
-    elseif moisense == MOI.MaxSense
-        return :Max
-    else
-        @assert moisense == MOI.FeasibilitySense
-        return :Feasibility
-    end
+    return MOI.get(model, MOI.ObjectiveSense())
+end
+
+"""
+    set_objective_sense(model::Model, sense::MathOptInterface.OptimizationSense)
+
+Sets the objective sense of the model to the given sense.
+"""
+function set_objective_sense(model::Model, sense::MOI.OptimizationSense)
+    MOI.set(model, MOI.ObjectiveSense(), sense)
 end
 
 # TODO(IainNZ): Document these too.

src/aff_expr.jl

@@ -295,16 +295,11 @@ function MOI.VectorAffineFunction(affs::Vector{AffExpr})
 end
 moi_function(a::Vector{<:GenericAffExpr}) = MOI.VectorAffineFunction(a)
 
-function set_objective(m::Model, sense::Symbol, a::AffExpr)
-    if sense == :Min
-        moisense = MOI.MinSense
-    else
-        @assert sense == :Max
-        moisense = MOI.MaxSense
-    end
-    MOI.set(m.moi_backend, MOI.ObjectiveSense(), moisense)
-    MOI.set(m.moi_backend, MOI.ObjectiveFunction{MOI.ScalarAffineFunction{Float64}}(), MOI.ScalarAffineFunction(a))
-    nothing
+function set_objective(model::Model, sense::MOI.OptimizationSense, a::AffExpr)
+    set_objective_sense(model, sense)
+    MOI.set(model, MOI.ObjectiveFunction{MOI.ScalarAffineFunction{Float64}}(),
+            MOI.ScalarAffineFunction(a))
+    return nothing
 end
 
 """

src/macros.jl

@@ -877,25 +877,29 @@ Constructs a vector of `QuadConstraint` objects. Similar to `@QuadConstraint`, e
 """ :(@QuadConstraints)
 
 
-
-
-
-
+# TODO: Add a docstring.
 macro objective(m, args...)
     m = esc(m)
     if length(args) != 2
-        # Either just an objective sene, or just an expression.
+        # Either just an objective sense, or just an expression.
         error("in @objective: needs three arguments: model, objective sense (Max or Min) and expression.")
     end
     sense, x = args
-    if sense == :Min || sense == :Max
-        sense = Expr(:quote,sense)
+    if sense == :Min
+        sense = MOI.MinSense
+    elseif sense == :Max
+        sense = MOI.MaxSense
+    else
+        # Refers to a variable that holds the sense.
+        # TODO: Better document this behavior and consider splitting some of the
+        # logic into a method that's reused by @NLobjective.
+        sense = esc(sense)
     end
     newaff, parsecode = parseExprToplevel(x, :q)
     code = quote
         q = Val{false}()
         $parsecode
-        set_objective($m, $(esc(sense)), $newaff)
+        set_objective($m, $sense, $newaff)
     end
     return assert_validmodel(m, macro_return(code, newaff))
 end
@@ -1433,13 +1437,18 @@ end
 
 # TODO: Add a docstring.
 macro NLobjective(m, sense, x)
-    if sense == :Min || sense == :Max
-        sense = Expr(:quote,sense)
+    if sense == :Min
+        sense = MOI.MinSense
+    elseif sense == :Max
+        sense = MOI.MaxSense
+    else
+        # Refers to a variable that holds the sense.
+        sense = esc(sense)
     end
     ex = gensym()
     code = quote
         $ex = $(processNLExpr(m, x))
-        set_objective($(esc(m)), $(esc(sense)), $ex)
+        set_objective($(esc(m)), $sense, $ex)
     end
     return assert_validmodel(esc(m), macro_return(code, ex))
 end

src/nlp.jl

@@ -9,15 +9,10 @@ mutable struct NonlinearExprData
     const_values::Vector{Float64}
 end
 
-function set_objective(m::Model, sense::Symbol, ex::NonlinearExprData)
+function set_objective(m::Model, sense::MOI.OptimizationSense,
+                       ex::NonlinearExprData)
     initNLP(m)
-    if sense == :Min
-        moisense = MOI.MinSense
-    else
-        @assert sense == :Max
-        moisense = MOI.MaxSense
-    end
-    MOI.set(m.moi_backend, MOI.ObjectiveSense(), moisense)
+    set_objective_sense(m, sense)
     m.nlp_data.nlobj = ex
     # TODO: what do we do about existing objectives in the MOI backend?
     return
@@ -1110,8 +1105,8 @@ function register(m::Model, s::Symbol, dimension::Integer, f::Function, ∇f::Fu
 end
 
 # TODO: Add a docstring.
-# Ex: set_NL_objective(model, :Min, :($x + $y^2))
-function set_NL_objective(model::Model, sense::Symbol, x)
+# Ex: set_NL_objective(model, MOI.MinSense, :($x + $y^2))
+function set_NL_objective(model::Model, sense::MOI.OptimizationSense, x)
     return set_objective(model, sense, NonlinearExprData(model, x))
 end
 

src/quad_expr.jl

@@ -221,16 +221,12 @@ function jump_function(model::AbstractModel, aff::MOI.ScalarQuadraticFunction)
     return QuadExpr(model, aff)
 end
 
-function set_objective(m::Model, sense::Symbol, a::QuadExpr)
-    if sense == :Min
-        moisense = MOI.MinSense
-    else
-        @assert sense == :Max
-        moisense = MOI.MaxSense
-    end
-    MOI.set(m.moi_backend, MOI.ObjectiveSense(), moisense)
-    MOI.set(m.moi_backend, MOI.ObjectiveFunction{MOI.ScalarQuadraticFunction{Float64}}(), MOI.ScalarQuadraticFunction(a))
-    nothing
+function set_objective(model::Model, sense::MOI.OptimizationSense, a::QuadExpr)
+    set_objective_sense(model, sense)
+    MOI.set(model,
+            MOI.ObjectiveFunction{MOI.ScalarQuadraticFunction{Float64}}(),
+            MOI.ScalarQuadraticFunction(a))
+    return nothing
 end
 
 """

src/variables.jl

@@ -244,17 +244,12 @@ function jump_function(model::AbstractModel, variable::MOI.SingleVariable)
     return VariableRef(model, variable)
 end
 
-function set_objective(m::Model, sense::Symbol, x::VariableRef)
-    # TODO: This code is repeated here, for GenericAffExpr, and for GenericQuadExpr.
-    if sense == :Min
-        moisense = MOI.MinSense
-    else
-        @assert sense == :Max
-        moisense = MOI.MaxSense
-    end
-    MOI.set(m.moi_backend, MOI.ObjectiveSense(), moisense)
-    MOI.set(m.moi_backend, MOI.ObjectiveFunction{MOI.SingleVariable}(),
-             MOI.SingleVariable(x))
+function set_objective(model::Model, sense::MOI.OptimizationSense,
+                       x::VariableRef)
+    set_objective_sense(model, sense)
+    MOI.set(model, MOI.ObjectiveFunction{MOI.SingleVariable}(),
+            MOI.SingleVariable(x))
+    return nothing
 end
 
 """

test/JuMPExtension.jl

@@ -15,13 +15,13 @@ mutable struct MyModel <: JuMP.AbstractModel
     nextconidx::Int                                 # Next constraint index is nextconidx+1
     constraints::Dict{Int, JuMP.AbstractConstraint} # Map conidx -> variable
     connames::Dict{Int, String}                     # Map varidx -> name
-    objectivesense::Symbol
+    objectivesense::MOI.OptimizationSense
     objective_function::JuMP.AbstractJuMPScalar
     obj_dict::Dict{Symbol, Any}                     # Same that JuMP.Model's field `obj_dict`
     function MyModel()
         new(0, Dict{Int, JuMP.AbstractVariable}(),   Dict{Int, String}(), # Variables
             0, Dict{Int, JuMP.AbstractConstraint}(), Dict{Int, String}(), # Constraints
-            :Min, zero(JuMP.GenericAffExpr{Float64, MyVariableRef}),
+            MOI.MinSense, zero(JuMP.GenericAffExpr{Float64, MyVariableRef}),
             Dict{Symbol, Any}())
     end
 end
@@ -145,11 +145,15 @@ function JuMP.constraint_object(cref::MyConstraintRef)
 end
 
 # Objective
-function JuMP.set_objective(m::MyModel, sense::Symbol, f::JuMP.AbstractJuMPScalar)
+function JuMP.set_objective(m::MyModel, sense::MOI.OptimizationSense,
+                            f::JuMP.AbstractJuMPScalar)
     m.objectivesense = sense
     m.objective_function = f
 end
-JuMP.objective_sense(m::MyModel) = m.objectivesense
+JuMP.objective_sense(model::MyModel) = model.objectivesense
+function JuMP.set_objective_sense(model::MyModel, sense)
+    model.objectivesense = sense
+end
 function JuMP.objective_function(m::MyModel, FT::Type)
     # InexactError should be thrown, this is needed in `objective.jl`
     if !(m.objective_function isa FT)

test/hygiene.jl

@@ -12,7 +12,7 @@ using Compat.Test
 import JuMP
 
 model = JuMP.Model()
-sense = :Min
+sense = JuMP.MathOptInterface.MinSense
 JuMP.@variable(model, x >= 0)
 r = 3:5
 JuMP.@variable(model, y[i=r] <= i)

test/nlp.jl

@@ -439,7 +439,7 @@
         model = Model()
         @variable(model, x)
         @variable(model, y)
-        JuMP.set_NL_objective(model, :Min, :($x^2 + $y^2))
+        JuMP.set_NL_objective(model, MOI.MinSense, :($x^2 + $y^2))
         JuMP.add_NL_constraint(model, :($x + $y <= 1))
         JuMP.add_NL_constraint(model, :($x + $y >= 1))
         JuMP.add_NL_constraint(model, :($x + $y == 1))

test/objective.jl

@@ -2,16 +2,22 @@ function objectives_test(ModelType::Type{<:JuMP.AbstractModel}, VariableRefType:
     AffExprType = JuMP.GenericAffExpr{Float64, VariableRefType}
     QuadExprType = JuMP.GenericQuadExpr{Float64, VariableRefType}
 
+    @testset "objective_sense set and get" begin
+        model = ModelType()
+        JuMP.set_objective_sense(model, MOI.FeasibilitySense)
+        @test JuMP.objective_sense(model) == MOI.FeasibilitySense
+    end
+
     @testset "SingleVariable objectives" begin
         m = ModelType()
         @variable(m, x)
 
         @objective(m, Min, x)
-        @test JuMP.objective_sense(m) == :Min
+        @test JuMP.objective_sense(m) == MOI.MinSense
         @test JuMP.objective_function(m, VariableRefType) == x
 
         @objective(m, Max, x)
-        @test JuMP.objective_sense(m) == :Max
+        @test JuMP.objective_sense(m) == MOI.MaxSense
         @test JuMP.objective_function(m, VariableRefType) == x
     end
 
@@ -20,11 +26,11 @@ function objectives_test(ModelType::Type{<:JuMP.AbstractModel}, VariableRefType:
         @variable(m, x)
 
         @objective(m, Min, 2x)
-        @test JuMP.objective_sense(m) == :Min
+        @test JuMP.objective_sense(m) == MOI.MinSense
         @test JuMP.isequal_canonical(JuMP.objective_function(m, AffExprType), 2x)
 
         @objective(m, Max, x + 3x + 1)
-        @test JuMP.objective_sense(m) == :Max
+        @test JuMP.objective_sense(m) == MOI.MaxSense
         @test JuMP.isequal_canonical(JuMP.objective_function(m, AffExprType), 4x + 1)
     end
 
@@ -33,7 +39,7 @@ function objectives_test(ModelType::Type{<:JuMP.AbstractModel}, VariableRefType:
         @variable(m, x)
 
         @objective(m, Min, x^2 + 2x)
-        @test JuMP.objective_sense(m) == :Min
+        @test JuMP.objective_sense(m) == MOI.MinSense
         @test JuMP.isequal_canonical(JuMP.objective_function(m, QuadExprType), x^2 + 2x)
         @test_throws InexactError JuMP.objective_function(m, AffExprType)
     end