overload MOI.set & supports for ConstraintAttribute in bridgeoptimizer

[guimarqu]

Hi, this PR follows my message in gitter.

I'm working on a package that creates variable/constraint attributes in the JuMP model. Then, another package retrieves these attributes through the MOI interface. In the second package, I overloaded MOI.set and MOI.get to copy/get variable & constraint attributes.

These are the definitions of MOI.set & MOI.get in my packages :

MOI.get(dest::MOIU.UniversalFallback, attr::BD.ConstraintDecomposition, ci::MOI.ConstraintIndex)
MOI.set(dest::MOIU.UniversalFallback, attr::BD.ConstraintDecomposition, ci::MOI.ConstraintIndex, annotation)

During the copy process, MOI.set of VariableAttribute receives a MOIU.CachingOptimizeras first argument whereas MOI.set of ConstraintAttribute receives a MOIU.Bridges.LazyBridgeOptimizer. Since LazyBridge is not a subtype of UniversalFallback, MOI throws an error. Looking in file bridgeoptimizer.jl, the following functions are defined for VariableAttribute but not for ConstraintAttribute.

function MOI.set(b::AbstractBridgeOptimizer,
                  attr::MOI.AbstractVariableAttribute,
                  index::MOI.Index, value)
    return MOI.set(b.model, attr, index, value)
end
function MOI.set(b::AbstractBridgeOptimizer,
                  attr::MOI.AbstractVariableAttribute,
                  indices::Vector{<:MOI.Index}, values::Vector)
    return MOI.set(b.model, attr, indices, values)
end

I overloaded MOI.set for ConstraintAttributesin bridge optimizer to pass the MOIU.CachingOptimizer as first argument.

I also overloaded MOI.support because there is no definition of it for ConstraintAttribute. However, I'm not sure of it is needed and I don't know how to test it. (I can get the attributes without overloading supports in my packages).

I can retrieve constraints attributes with these changes but is this the reason of my bug?

[codecov-io]

Codecov Report

Merging #699 into master will decrease coverage by 0.16%.
The diff coverage is 88.23%.

@@            Coverage Diff            @@
##           master    #699      +/-   ##
=========================================
- Coverage   95.26%   95.1%   -0.17%     
=========================================
  Files          54      54              
  Lines        5597    5618      +21     
=========================================
+ Hits         5332    5343      +11     
- Misses        265     275      +10

Impacted Files	Coverage Δ
src/Bridges/bridgeoptimizer.jl	97.6% <88.23%> (-1.51%)	⬇️
src/modifications.jl	50% <0%> (-7.15%)	⬇️
src/Bridges/slackbridge.jl	94.82% <0%> (-5.18%)	⬇️
src/sets.jl	95.74% <0%> (-4.26%)	⬇️
src/Bridges/detbridge.jl	98.75% <0%> (-1.25%)	⬇️
src/Test/intlinear.jl	98.95% <0%> (-0.52%)	⬇️

---

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 e7b75f8...1f1503f. Read the comment docs.

[blegat]

These two methods are indeed missing, thanks for the PR! However, the implementation you suggest will only work if the constraint is not bridged, the implementation should branch on whether it is bridged or not, see, e.g.:
https://github.com/JuliaOpt/MathOptInterface.jl/blob/f18603addbcab092aeda76df1ff9112a1d69e6c2/src/Bridges/bridgeoptimizer.jl#L225-L238
To test the implementation, you could implement starting values for one of the bridges (see #684) and test that get, set and supports work correctly for starting values of constraints bridges using this bridge.
Note that MOI.get should be modified as
https://github.com/JuliaOpt/MathOptInterface.jl/blob/f18603addbcab092aeda76df1ff9112a1d69e6c2/src/Bridges/bridgeoptimizer.jl#L233
should only be called for names.
https://github.com/JuliaOpt/MathOptInterface.jl/blob/f18603addbcab092aeda76df1ff9112a1d69e6c2/src/Bridges/bridgeoptimizer.jl#L230
should be replaced by if !(attr isa MOI.ConstraintName) (note that the bridged field is going away with #523).
All constraint attributes (except constraint names of bridged constraints) should be transferred to b.model either directly if the constraint is not bridged or indirectly if it is bridged (similarly to what is done with ConstraintPrimal and ConstraintDual).

[guimarqu]

So now I have to implement starting values for constraint bridges and the test should be something like

MOI.set(mock_optimizer, ConstraintPrimalStart(), index_of_a_bridged_constraint, value)

and when I'll do

MOI.get(mock_optimizer, ConstraintPrimalStart(), index_of_a_bridged_constraint)

I should be able to retrieve value.

Did I understand ? Where should I do the tests ?

[blegat]

You can for instance implement it to FlipSign and then add a testset here:
https://github.com/JuliaOpt/MathOptInterface.jl/blob/f18603addbcab092aeda76df1ff9112a1d69e6c2/test/bridge.jl#L409
Trying set and get looks good, you should also get it from the mock so check that is was not just set to the bridged field but it was transmitted to the solver. Also check supports

[blegat]

You can leave <:CI

[guimarqu]

Julia throws an "ambiguous name" method error if I leave <:CI.

[guimarqu]

Hi,

I try writing a basic test :

struct FlipSign <: MOI.AbstractConstraintAttribute end

MOI.set(bridged_mock, FlipSign(), ci, true)
@test MOI.get(bridged_mock, FlipSign(), ci)

The constraint is bridged so it calls MOI.set(bridged_mock, FlipSign(), bridge(bridged_mock, ci), true).

But bridge(bridged_mock, ci) returns a MathOptInterface.Bridges.GreaterToLessBridge{Float64,MathOptInterface.ScalarAffineFunction{Float64}}.

I'm stuck because I'm not sure what is going on here. What should I do with this constraint bridge? Should I overload MOI.set for all types of constraint bridge?

[blegat]

Yes an set method should be added for every bridge.
For this PR I suggest to implement ConstraintPrimalStart and ConstraintDualStart for one bridge only. This is enough to test the changes of this PR

[blegat]

Let's add the tests in a separate PR (#719), thanks for the contribution !