WIP: VectorAffineBridge #413
Conversation
src/Bridges/vaftosafbridge.jl
Outdated
| saf = functions[term.output_index] | ||
| push!(saf.terms, term.scalar_term) | ||
| end | ||
| ci = CI{MOI.ScalarAffineFunction{T}, S}[] |
src/Bridges/vaftosafbridge.jl
Outdated
| getscalarset(T, ::Type{MOI.Nonnegatives}) = MOI.GreaterThan{T} | ||
|
|
||
|
|
||
| MOI.supportsconstraint(::Type{VectorAffineBridge{T,S}}, ::Type{MOI.VectorAffineFunction{T}}, ::Type{S}) where {T,S<:VectorAffineBridgeSets} = true |
There was a problem hiding this comment.
S is different here. The first S is the scalar set and the second is the vector set
|
Looks good ! We might wonder whether we should add the bridge with or without the type |
src/Bridges/vaftosafbridge.jl
Outdated
| end | ||
| ci = CI{MOI.ScalarAffineFunction{T}, S}[] | ||
| for (func, set) in zip(functions, sets) | ||
| push!(ci, MOI.addconstraint!(model, func, set)) |
There was a problem hiding this comment.
This should use MOI.add_constraints! so that solvers can provide an efficient implementation. @joaquimg
Codecov Report
@@ Coverage Diff @@
## master #413 +/- ##
==========================================
- Coverage 95.26% 94.31% -0.95%
==========================================
Files 42 43 +1
Lines 4962 5013 +51
==========================================
+ Hits 4727 4728 +1
- Misses 235 285 +50
Continue to review full report at Codecov.
|
| sets = [ScalarSet(-constant) for constant in func.constants] | ||
| @assert MOI.dimension(set) == length(sets) | ||
| functions = [MOI.ScalarAffineFunction{T}(MOI.ScalarAffineTerm{T}[], zero(T)) | ||
| for i in 1:MOI.dimension(set)] |
There was a problem hiding this comment.
Cosmetic/Subjective: I would indent to start for just past the [
There was a problem hiding this comment.
Ha ... if it was up to me, anything other than 4 space indents would be a jailable offense
There was a problem hiding this comment.
indenting is where coder can express their artistic inner-self :D
More seriously, I was confused when reading the line as I thought it was a block (e.g. like for, begin, if, ...) instead of a line-break.
I usually do 4-spaces for blocks and for line-breaks I align with parentheses, brackets, ...
| for term in func.terms | ||
| scalar_function = functions[term.output_index] | ||
| push!(scalar_function.terms, term.scalar_term) | ||
| end |
There was a problem hiding this comment.
These lines are equivalent to functions = MOIU.eachscalar(func)[1:MOI.dimension(set)]. The advantage of this is that it could also work with VectorQuadraticFunction without the need to do any change in the bridge
| MOI.canget(model, MOI.ConstraintSet(), CI{MOI.ScalarAffineFunction{T}, S}) | ||
| end | ||
| function MOI.get(model::MOI.ModelLike, ::MOI.ConstraintPrimal, bridge::VectorToScalarBridge) | ||
| primals = fill(0.0, length(bridge.constraints)) |
There was a problem hiding this comment.
zeros(T, length(bridge.constraints))
| MOI.canget(::MOI.ModelLike, ::MOI.ConstraintFunction, ::Type{<:VectorToScalarBridge}) = true | ||
| function MOI.get(model::MOI.ModelLike, ::MOI.ConstraintFunction, | ||
| bridge::VectorToScalarBridge{T, S}) where {T, S} | ||
| terms = VectorAffineTerm{T}[] |
There was a problem hiding this comment.
moivcat can be helpful here. Using moivcat will help adding support for quadratic functions
| - `MOI.Nonnegatives` =>`MOI.GreaterThan{T}` | ||
| - `MOI.Nonpositives` =>`MOI.LessThan{T}` | ||
| """ | ||
| struct VectorToScalarBridge{T, S} <: AbstractBridge |
There was a problem hiding this comment.
Would be nice to have
struct VectorToScalarBridge{T, F, S} <: AbstractBridge
constriants::Vector{CI{F, S}}
endwhere F can also be, e.g. SingleVariable and ScalarQuadraticFunction.
You can leave it for future work as the tools needed are being added in #455
|
Looks good ! Just missing a few tests :) |
This PR adds a bridge between
VectorAffineFunction-in-{Zeros, Nonnegatives, Nonpositives}and multipleScalarAffineFunction-in-{EqualTo, LessThan, GreaterThan}.The biggest target for this is LQOI. We should be able to remove a large chunk of code which effectively implements this bridge anyway. I commented out all of the
VectorAffineFunctioncode in LQOI, and then ran the GLPK tests withMOIB.VectorToScalar{Float64}(GLPK.Optimizer())and everything passed!Just playing around trying to understand the bridge code.Untested atm.
@blegat is this how it should be?