Fixes for copy and file write (#158)

* simplify affine affine and add API for direct usage * simplify add constraint * fix format * Fix to allow copy_to * activate more tests * fix format * add test for coverage --------- Co-authored-by: Joaquim Garcia <[email protected]>
jump-dev · Aug 6, 2024 · 9b3454d · 9b3454d
1 parent 243beb3
commit 9b3454d
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 75 deletions.
diff --git a/src/MOI_wrapper.jl b/src/MOI_wrapper.jl
@@ -140,9 +140,13 @@ end
 # Variables
 #
 
-# TODO: This is not correct
 function MOI.is_valid(model::Optimizer, vi::MOI.VariableIndex)
-    return MOI.is_valid(model.optimizer, vi)
+    if haskey(model.variables, vi)
+        return true
+    elseif haskey(model.parameters, p_idx(vi))
+        return true
+    end
+    return false
 end
 
 function MOI.supports(
@@ -304,11 +308,8 @@ function _delete_variable_index_constraint(
     value,
 )
     inner = d[F, S]
-    for k in keys(inner)
-        if k.value == value
-            delete!(inner, k)
-        end
-    end
+    key = MOI.ConstraintIndex{F,S}(value)
+    delete!(inner, key)
     return
 end
 
@@ -600,7 +601,6 @@ function _add_constraint_with_parameters_on_function(
     set::S,
 ) where {T,S}
     pf = ParametricAffineFunction(f)
-    _cache_set_constant!(pf, set)
     if model.constraints_interpretation == ONLY_BOUNDS
         if length(pf.v) == 1 && isone(MOI.coefficient(pf.v[]))
             poi_ci = _add_vi_constraint(model, pf, set)
@@ -610,24 +610,25 @@ function _add_constraint_with_parameters_on_function(
             )
         end
     elseif model.constraints_interpretation == ONLY_CONSTRAINTS
-        poi_ci = _add_saf_constraint(model, pf, set)
+        poi_ci = MOI.add_constraint(model, pf, set)
     elseif model.constraints_interpretation == BOUNDS_AND_CONSTRAINTS
         if length(pf.v) == 1 && isone(MOI.coefficient(pf.v[]))
             poi_ci = _add_vi_constraint(model, pf, set)
         else
-            poi_ci = _add_saf_constraint(model, pf, set)
+            poi_ci = MOI.add_constraint(model, pf, set)
         end
     end
     return poi_ci
 end
 
-function _add_saf_constraint(
+function MOI.add_constraint(
     model::Optimizer,
     pf::ParametricAffineFunction{T},
     set::S,
 ) where {T,S}
+    _cache_set_constant!(pf, set)
     _update_cache!(pf, model)
-    inner_ci = MOI.Utilities.normalize_and_add_constraint(
+    inner_ci = MOI.add_constraint(
         model.optimizer,
         MOI.ScalarAffineFunction{T}(pf.v, 0.0),
         _set_with_new_constant(set, pf.current_constant),
@@ -648,8 +649,9 @@ function _add_vi_constraint(
     pf::ParametricAffineFunction{T},
     set::S,
 ) where {T,S}
+    _cache_set_constant!(pf, set)
     _update_cache!(pf, model)
-    inner_ci = MOI.Utilities.normalize_and_add_constraint(
+    inner_ci = MOI.add_constraint(
         model.optimizer,
         pf.v[].variable,
         _set_with_new_constant(set, pf.current_constant),
@@ -725,13 +727,10 @@ function _add_constraint_with_parameters_on_function(
     _update_cache!(pf, model)
 
     func = _current_function(pf)
-    f_quad = if !_is_affine(func)
+    if !_is_affine(func)
         fq = func
-        inner_ci = MOI.Utilities.normalize_and_add_constraint(
-            model.optimizer,
-            fq,
-            s,
-        )
+        inner_ci =
+            MOI.Utilities.normalize_and_add_constraint(model.optimizer, fq, s)
         model.last_quad_add_added += 1
         outer_ci = MOI.ConstraintIndex{MOI.ScalarQuadraticFunction{T},S}(
             model.last_quad_add_added,
@@ -740,11 +739,8 @@ function _add_constraint_with_parameters_on_function(
         model.constraint_outer_to_inner[outer_ci] = inner_ci
     else
         fa = MOI.ScalarAffineFunction(func.affine_terms, func.constant)
-        inner_ci = MOI.Utilities.normalize_and_add_constraint(
-            model.optimizer,
-            fa,
-            s,
-        )
+        inner_ci =
+            MOI.Utilities.normalize_and_add_constraint(model.optimizer, fa, s)
         model.last_quad_add_added += 1
         outer_ci = MOI.ConstraintIndex{MOI.ScalarQuadraticFunction{T},S}(
             model.last_quad_add_added,
@@ -1009,15 +1005,19 @@ function MOI.get(
 end
 
 function MOI.get(model::Optimizer, ::MOI.NumberOfVariables)
-    return length(model.parameters) + length(model.variables)
+    return MOI.get(model, NumberOfPureVariables()) +
+           MOI.get(model, NumberOfParameters())
 end
 
 function MOI.get(model::Optimizer, ::MOI.NumberOfConstraints{F,S}) where {S,F}
     return length(model.constraint_outer_to_inner[F, S])
 end
 
 function MOI.get(model::Optimizer, ::MOI.ListOfVariableIndices)
-    return MOI.get(model.optimizer, MOI.ListOfVariableIndices())
+    return vcat(
+        MOI.get(model, ListOfPureVariableIndices()),
+        v_idx.(MOI.get(model, ListOfParameterIndices())),
+    )
 end
 
 function MOI.get(model::Optimizer, ::MOI.ListOfConstraintTypesPresent)
@@ -1113,16 +1113,28 @@ end
 # Special Attributes
 #
 
+struct NumberOfPureVariables <: MOI.AbstractModelAttribute end
+
+function MOI.get(model::Optimizer, ::NumberOfPureVariables)
+    return length(model.variables)
+end
+
 struct ListOfPureVariableIndices <: MOI.AbstractModelAttribute end
 
 function MOI.get(model::Optimizer, ::ListOfPureVariableIndices)
-    return collect(keys(model.variables))
+    return collect(keys(model.variables))::Vector{MOI.VariableIndex}
+end
+
+struct NumberOfParameters <: MOI.AbstractModelAttribute end
+
+function MOI.get(model::Optimizer, ::NumberOfParameters)
+    return length(model.parameters)
 end
 
 struct ListOfParameterIndices <: MOI.AbstractModelAttribute end
 
 function MOI.get(model::Optimizer, ::ListOfParameterIndices)
-    return collect(keys(model.parameters))
+    return collect(keys(model.parameters))::Vector{ParameterIndex}
 end
 
 """

diff --git a/src/ParametricOptInterface.jl b/src/ParametricOptInterface.jl
@@ -28,6 +28,10 @@ function p_idx(vi::MOI.VariableIndex)::ParameterIndex
     return ParameterIndex(vi.value - PARAMETER_INDEX_THRESHOLD)
 end
 
+function v_idx(pi::ParameterIndex)::MOI.VariableIndex
+    return MOI.VariableIndex(pi.index + PARAMETER_INDEX_THRESHOLD)
+end
+
 function p_val(vi::MOI.VariableIndex)::Int64
     return vi.value - PARAMETER_INDEX_THRESHOLD
 end

diff --git a/src/parametric_functions.jl b/src/parametric_functions.jl
@@ -1,5 +1,21 @@
+abstract type ParametricFunction{T} end
 
-mutable struct ParametricQuadraticFunction{T}
+function _cache_set_constant!(
+    f::ParametricFunction{T},
+    s::Union{MOI.LessThan{T},MOI.GreaterThan{T},MOI.EqualTo{T}},
+) where {T}
+    f.set_constant = MOI.constant(s)
+    return
+end
+
+function _cache_set_constant!(
+    ::ParametricFunction{T},
+    ::MOI.AbstractScalarSet,
+) where {T}
+    return
+end
+
+mutable struct ParametricQuadraticFunction{T} <: ParametricFunction{T}
     # helper to efficiently update affine terms 
     affine_data::Dict{MOI.VariableIndex,T}
     affine_data_np::Dict{MOI.VariableIndex,T}
@@ -147,21 +163,6 @@ function _current_function(f::ParametricQuadraticFunction{T}) where {T}
     return MOI.ScalarQuadraticFunction{T}(f.vv, affine, f.current_constant)
 end
 
-function _cache_set_constant!(
-    f::ParametricQuadraticFunction{T},
-    s::Union{MOI.LessThan{T},MOI.GreaterThan{T},MOI.EqualTo{T}},
-) where {T}
-    f.set_constant = MOI.constant(s)
-    return
-end
-
-function _cache_set_constant!(
-    ::ParametricQuadraticFunction{T},
-    ::MOI.AbstractScalarSet,
-) where {T}
-    return
-end
-
 function _parametric_constant(
     model,
     f::ParametricQuadraticFunction{T},
@@ -263,7 +264,7 @@ function _update_cache!(f::ParametricQuadraticFunction{T}, model) where {T}
     return nothing
 end
 
-mutable struct ParametricAffineFunction{T}
+mutable struct ParametricAffineFunction{T} <: ParametricFunction{T}
     # constant * parameter
     p::Vector{MOI.ScalarAffineTerm{T}}
     # constant * variable
@@ -278,7 +279,21 @@ end
 
 function ParametricAffineFunction(f::MOI.ScalarAffineFunction{T}) where {T}
     v, p = _split_affine_terms(f.terms)
-    return ParametricAffineFunction{T}(p, v, f.constant, zero(T), zero(T))
+    return ParametricAffineFunction(p, v, f.constant)
+end
+
+function ParametricAffineFunction(
+    terms_p::Vector{MOI.ScalarAffineTerm{T}},
+    terms_v::Vector{MOI.ScalarAffineTerm{T}},
+    constant::T,
+) where {T}
+    return ParametricAffineFunction{T}(
+        terms_p,
+        terms_v,
+        constant,
+        zero(T),
+        zero(T),
+    )
 end
 
 function _split_affine_terms(terms::Vector{MOI.ScalarAffineTerm{T}}) where {T}
@@ -322,21 +337,6 @@ function _current_function(f::ParametricAffineFunction{T}) where {T}
     return MOI.ScalarAffineFunction{T}(f.v, f.current_constant)
 end
 
-function _cache_set_constant!(
-    f::ParametricAffineFunction{T},
-    s::Union{MOI.LessThan{T},MOI.GreaterThan{T},MOI.EqualTo{T}},
-) where {T}
-    f.set_constant = MOI.constant(s)
-    return
-end
-
-function _cache_set_constant!(
-    f::ParametricAffineFunction{T},
-    s::MOI.AbstractScalarSet,
-) where {T}
-    return
-end
-
 function _parametric_constant(model, f::ParametricAffineFunction{T}) where {T}
     # do not add set_function here
     param_constant = f.c

diff --git a/test/moi_tests.jl b/test/moi_tests.jl
@@ -17,8 +17,13 @@ function test_basic_tests()
     MOI.set(optimizer, MOI.Silent(), true)
     x = MOI.add_variables(optimizer, 2)
     y, cy = MOI.add_constrained_variable(optimizer, MOI.Parameter(0.0))
+    @test MOI.is_valid(optimizer, x[1])
+    @test MOI.is_valid(optimizer, y)
+    @test MOI.get(optimizer, POI.ListOfPureVariableIndices()) == x
+    @test MOI.get(optimizer, MOI.ListOfVariableIndices()) == [x[1], x[2], y]
     z = MOI.VariableIndex(4)
     cz = MOI.ConstraintIndex{MOI.VariableIndex,MOI.Parameter{Float64}}(4)
+    @test !MOI.is_valid(optimizer, z)
     for x_i in x
         MOI.add_constraint(optimizer, x_i, MOI.GreaterThan(0.0))
     end
@@ -208,9 +213,6 @@ function test_moi_glpk()
         exclude = [
             # GLPK returns INVALID_MODEL instead of INFEASIBLE
             "test_constraint_ZeroOne_bounds_3",
-            # Upstream issue: https://github.com/jump-dev/MathOptInterface.jl/issues/1431
-            "test_model_LowerBoundAlreadySet",
-            "test_model_UpperBoundAlreadySet",
         ],
     )
     return
@@ -259,18 +261,7 @@ function test_moi_ipopt()
             #  - Excluded because Ipopt returns LOCALLY_INFEASIBLE instead of
             #    INFEASIBLE
             "INFEASIBLE",
-            "test_conic_linear_INFEASIBLE",
-            "test_conic_linear_INFEASIBLE_2",
             "test_solve_DualStatus_INFEASIBILITY_CERTIFICATE_",
-            #  - Excluded due to upstream issue
-            "test_model_LowerBoundAlreadySet",
-            "test_model_UpperBoundAlreadySet",
-            #  - CachingOptimizer does not throw if optimizer not attached
-            "test_model_copy_to_UnsupportedAttribute",
-            "test_model_copy_to_UnsupportedConstraint",
-            #  - POI throws a ErrorException if user tries to modify parametric
-            #    functions
-            "test_objective_get_ObjectiveFunction_ScalarAffineFunction",
         ],
     )
     return