diff --git a/src/Utilities/cachingoptimizer.jl b/src/Utilities/cachingoptimizer.jl index 7572eb4cd4..672900c906 100644 --- a/src/Utilities/cachingoptimizer.jl +++ b/src/Utilities/cachingoptimizer.jl @@ -864,15 +864,34 @@ function _get_model_attribute( ) end +function _throw_if_get_attribute_not_allowed( + model::CachingOptimizer, + attr; + needs_optimizer_map::Bool, +) + # If the state(model) == EMPTY_OPTIMIZER, then + # `model.model_to_optimizer_map[index]` might be empty (because copy_to + # has not been called yet), or it might be full, if optimize!(dest, src) + # did not leave a copy in `dest`. + missing_map = needs_optimizer_map && isempty(model.model_to_optimizer_map) + if state(model) == NO_OPTIMIZER || missing_map + msg = + "Cannot query $(attr) from `Utilities.CachingOptimizer` " * + "because no optimizer is attached (the state is `$(state(model))`)." + throw(MOI.GetAttributeNotAllowed(attr, msg)) + end + return +end + function MOI.get(model::CachingOptimizer, attr::MOI.AbstractModelAttribute) if !MOI.is_set_by_optimize(attr) return MOI.get(model.model_cache, attr) - elseif state(model) == NO_OPTIMIZER - error( - "Cannot query $(attr) from caching optimizer because no " * - "optimizer is attached.", - ) end + _throw_if_get_attribute_not_allowed( + model, + attr; + needs_optimizer_map = false, + ) return _get_model_attribute(model, attr) end @@ -901,25 +920,16 @@ function MOI.get( attr::Union{MOI.AbstractVariableAttribute,MOI.AbstractConstraintAttribute}, index::MOI.Index, ) - if MOI.is_set_by_optimize(attr) - if state(model) == NO_OPTIMIZER - error( - "Cannot query $(attr) from caching optimizer because no " * - "optimizer is attached.", - ) - end - return map_indices( - model.optimizer_to_model_map, - attr, - MOI.get( - model.optimizer, - attr, - model.model_to_optimizer_map[index], - )::MOI.attribute_value_type(attr), - ) - else + if !MOI.is_set_by_optimize(attr) return MOI.get(model.model_cache, attr, index) end + _throw_if_get_attribute_not_allowed(model, attr; needs_optimizer_map = true) + value = MOI.get( + model.optimizer, + attr, + model.model_to_optimizer_map[index], + )::MOI.attribute_value_type(attr) + return map_indices(model.optimizer_to_model_map, attr, value) end function MOI.get( @@ -927,25 +937,16 @@ function MOI.get( attr::Union{MOI.AbstractVariableAttribute,MOI.AbstractConstraintAttribute}, indices::Vector{<:MOI.Index}, ) - if MOI.is_set_by_optimize(attr) - if state(model) == NO_OPTIMIZER - error( - "Cannot query $(attr) from caching optimizer because no " * - "optimizer is attached.", - ) - end - return map_indices( - model.optimizer_to_model_map, - attr, - MOI.get( - model.optimizer, - attr, - map(index -> model.model_to_optimizer_map[index], indices), - )::Vector{<:MOI.attribute_value_type(attr)}, - ) - else + if !MOI.is_set_by_optimize(attr) return MOI.get(model.model_cache, attr, indices) end + _throw_if_get_attribute_not_allowed(model, attr; needs_optimizer_map = true) + value = MOI.get( + model.optimizer, + attr, + map(Base.Fix1(getindex, model.model_to_optimizer_map), indices), + )::Vector{<:MOI.attribute_value_type(attr)} + return map_indices(model.optimizer_to_model_map, attr, value) end ### @@ -961,12 +962,7 @@ function MOI.get( attr::MOI.ConstraintPrimal, index::MOI.ConstraintIndex, ) - if state(model) == NO_OPTIMIZER - error( - "Cannot query $(attr) from caching optimizer because no " * - "optimizer is attached.", - ) - end + _throw_if_get_attribute_not_allowed(model, attr; needs_optimizer_map = true) try return MOI.get( model.optimizer, @@ -987,12 +983,7 @@ function MOI.get( attr::MOI.ConstraintPrimal, indices::Vector{<:MOI.ConstraintIndex}, ) - if state(model) == NO_OPTIMIZER - error( - "Cannot query $(attr) from caching optimizer because no " * - "optimizer is attached.", - ) - end + _throw_if_get_attribute_not_allowed(model, attr; needs_optimizer_map = true) try return MOI.get( model.optimizer, @@ -1055,16 +1046,15 @@ function MOI.set( end function MOI.get(model::CachingOptimizer, attr::MOI.AbstractOptimizerAttribute) - if state(model) == NO_OPTIMIZER - # TODO: Copyable attributes (e.g., `Silent`, `TimeLimitSec`, - # `NumberOfThreads`) should also be stored in the cache so we could - # return the value stored in the cache instead. However, for - # non-copyable attributes( e.g. `SolverName`) the error is appropriate. - error( - "Cannot query $(attr) from caching optimizer because no " * - "optimizer is attached.", - ) - end + # TODO: Copyable attributes (e.g., `Silent`, `TimeLimitSec`, + # `NumberOfThreads`) should also be stored in the cache so we could + # return the value stored in the cache instead. However, for + # non-copyable attributes( e.g. `SolverName`) the error is appropriate. + _throw_if_get_attribute_not_allowed( + model, + attr; + needs_optimizer_map = false, + ) return map_indices( model.optimizer_to_model_map, attr, diff --git a/src/attributes.jl b/src/attributes.jl index 6fe2291692..c30f67329b 100644 --- a/src/attributes.jl +++ b/src/attributes.jl @@ -2242,14 +2242,23 @@ end """ is_set_by_optimize(::AnyAttribute) -Return a `Bool` indicating whether the value of the attribute is modified -during an [`optimize!`](@ref) call, that is, the attribute is used to query -the result of the optimization. +Return a `Bool` indicating whether the value of the attribute is set during an +[`optimize!`](@ref) call, that is, the attribute is used to query the result of +the optimization. -## Important note when defining new attributes +If an attibute can be set by the user, define [`is_copyable`](@ref) instead. + +An attribute cannot be both [`is_copyable`](@ref) and `is_set_by_optimize`. + +## Default fallback This function returns `false` by default so it should be implemented for -attributes that are modified by [`optimize!`](@ref). +attributes that are set by [`optimize!`](@ref). + +## Undefined behavior + +Querying the value of the attribute that `is_set_by_optimize` before a call to +[`optimize!`](@ref) is undefined and depends on solver-specific behavior. """ is_set_by_optimize(::AnyAttribute) = false @@ -2287,27 +2296,20 @@ end Return a `Bool` indicating whether the value of the attribute may be copied during [`copy_to`](@ref) using [`set`](@ref). -## Important note when defining new attributes - -By default `is_copyable(attr)` returns `!is_set_by_optimize(attr)`. A specific -method should be defined for attributes which are copied indirectly during -[`copy_to`](@ref). For instance, both `is_copyable` and -[`is_set_by_optimize`](@ref) return `false` for the following attributes: - -* [`ListOfOptimizerAttributesSet`](@ref), [`ListOfModelAttributesSet`](@ref), - [`ListOfConstraintAttributesSet`](@ref) and - [`ListOfVariableAttributesSet`](@ref). -* [`SolverName`](@ref) and [`RawSolver`](@ref): these attributes cannot be set. -* [`NumberOfVariables`](@ref) and [`ListOfVariableIndices`](@ref): these - attributes are set indirectly by [`add_variable`](@ref) and - [`add_variables`](@ref). -* [`ObjectiveFunctionType`](@ref): this attribute is set indirectly when setting - the [`ObjectiveFunction`](@ref) attribute. -* [`NumberOfConstraints`](@ref), [`ListOfConstraintIndices`](@ref), - [`ListOfConstraintTypesPresent`](@ref), [`CanonicalConstraintFunction`](@ref), - [`ConstraintFunction`](@ref) and [`ConstraintSet`](@ref): - these attributes are set indirectly by - [`add_constraint`](@ref) and [`add_constraints`](@ref). +If an attribute `is_copyable`, then it cannot be modified by the optimizer, and +[`get`](@ref) must always return the value that was [`set`](@ref) by the user. + +If an attibute is the result of an optimization, define +[`is_set_by_optimize`](@ref) instead. + +An attribute cannot be both [`is_set_by_optimize`](@ref) and `is_copyable`. + +## Default fallback + +By default `is_copyable(attr)` returns `!is_set_by_optimize(attr)`, which is +most probably `true`. + +If an attribute should not be copied, define `is_copyable(::MyAttribute) = false`. """ function is_copyable(attr::AnyAttribute) return !is_set_by_optimize(attr) diff --git a/test/Utilities/cachingoptimizer.jl b/test/Utilities/cachingoptimizer.jl index f37871867a..534168cfb5 100644 --- a/test/Utilities/cachingoptimizer.jl +++ b/test/Utilities/cachingoptimizer.jl @@ -179,13 +179,14 @@ function test_default_attributes() @test MOI.get(model, MOI.PrimalStatus()) == MOI.NO_SOLUTION @test MOI.get(model, MOI.DualStatus()) == MOI.NO_SOLUTION x = MOI.add_variables(model, 2) - attr = MOI.VariablePrimal() - exception = ErrorException( - "Cannot query $(attr) from caching optimizer because no optimizer" * - " is attached.", + @test_throws( + MOI.GetAttributeNotAllowed{MOI.VariablePrimal}, + MOI.get(model, MOI.VariablePrimal(), x[1]), + ) + @test_throws( + MOI.GetAttributeNotAllowed{MOI.VariablePrimal}, + MOI.get(model, MOI.VariablePrimal(), x), ) - @test_throws exception MOI.get(model, MOI.VariablePrimal(), x[1]) - @test_throws exception MOI.get(model, MOI.VariablePrimal(), x) for attr in ( MOI.SolverName(), MOI.Silent(), @@ -195,10 +196,7 @@ function test_default_attributes() MOI.ResultCount(), ) @test_throws( - ErrorException( - "Cannot query $(attr) from caching optimizer because no " * - "optimizer is attached.", - ), + MOI.GetAttributeNotAllowed{typeof(attr)}, MOI.get(model, attr), ) end @@ -450,7 +448,7 @@ function test_CachingOptimizer_MANUAL_mode() typeof(v), ) MOI.set(m, MOIU.AttributeFromOptimizer(MOI.VariablePrimal()), v, 3.0) - + MOI.set(s, MOI.TerminationStatus(), MOI.OPTIMAL) MOI.optimize!(m) @test MOI.get(m, MOI.VariablePrimal(), v) == 3.0 @@ -828,6 +826,8 @@ MOI.get(::_GetFallbackModel1310, ::MOI.PrimalStatus) = MOI.FEASIBLE_POINT MOI.get(::_GetFallbackModel1310, ::MOI.DualStatus) = MOI.FEASIBLE_POINT +MOI.get(::_GetFallbackModel1310, ::MOI.TerminationStatus) = MOI.OTHER_ERROR + MOI.get(::_GetFallbackModel1310, ::MOI.ResultCount) = 1 function MOI.optimize!(::_GetFallbackModel1310, model::MOI.ModelLike) @@ -887,8 +887,14 @@ function test_ConstraintPrimal_fallback_error() ) x = MOI.add_variable(model) c = MOI.add_constraint(model, x, MOI.GreaterThan(1.0)) - @test_throws(ErrorException, MOI.get(model, MOI.ConstraintPrimal(), c)) - @test_throws(ErrorException, MOI.get(model, MOI.ConstraintPrimal(), [c])) + @test_throws( + MOI.GetAttributeNotAllowed{MOI.ConstraintPrimal}, + MOI.get(model, MOI.ConstraintPrimal(), c), + ) + @test_throws( + MOI.GetAttributeNotAllowed{MOI.ConstraintPrimal}, + MOI.get(model, MOI.ConstraintPrimal(), [c]), + ) return end