Skip to content

Commit

Permalink
Fix method ambiguities in JuMP (#3092)
Browse files Browse the repository at this point in the history
  • Loading branch information
odow authored Sep 28, 2022
1 parent 2a587d0 commit f5749e5
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 8 deletions.
1 change: 1 addition & 0 deletions src/Containers/DenseAxisArray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ function Base.similar(
) where {T,N,Ax,S}
return construct_undef_array(S, axes)
end

# Avoid conflict with method defined in Julia Base when the axes of the
# `DenseAxisArray` are all `Base.OneTo`:
function Base.similar(
Expand Down
17 changes: 17 additions & 0 deletions src/aff_expr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,14 @@ end

# With two factors.

function add_to_expression!(
expr::GenericAffExpr{C,V},
α::_Constant,
β::_Constant,
) where {C,V}
return add_to_expression!(expr, *(α, β))
end

function add_to_expression!(
aff::GenericAffExpr{C,V},
new_coef::_Constant,
Expand All @@ -445,6 +453,15 @@ function add_to_expression!(
return add_to_expression!(aff, new_coef, new_var)
end

function add_to_expression!(
aff::GenericAffExpr{C,V},
new_coef::V,
new_var::V,
) where {C,V<:_Constant}
_add_or_set!(aff.terms, new_var, convert(C, _constant_to_number(new_coef)))
return aff
end

function add_to_expression!(
aff::GenericAffExpr{S,V},
coef::_Constant,
Expand Down
8 changes: 0 additions & 8 deletions src/mutable_arithmetics.jl
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,6 @@ function _MA.operate!(
) where {N}
return _add_sub_mul_reorder!(op, expr, x, y, z, other_args...)
end
# This method is missing for both affine and quadratic expressions.
function add_to_expression!(
expr::_GenericAffOrQuadExpr,
α::_Constant,
β::_Constant,
)
return add_to_expression!(expr, *(α, β))
end

const _AffineLike = Union{AbstractVariableRef,GenericAffExpr,_Constant}

Expand Down
25 changes: 25 additions & 0 deletions src/quad_expr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,14 @@ end

# With two factors.

function add_to_expression!(
expr::GenericQuadExpr{C,V},
α::_Constant,
β::_Constant,
) where {C,V}
return add_to_expression!(expr, *(α, β))
end

function add_to_expression!(
quad::GenericQuadExpr{C,V},
new_coef::_Constant,
Expand All @@ -335,6 +343,15 @@ function add_to_expression!(
return add_to_expression!(quad, new_coef, new_var)
end

function add_to_expression!(
quad::GenericQuadExpr{C,V},
new_coef::V,
new_var::V,
) where {C,V<:Union{Number,LinearAlgebra.UniformScaling}}
add_to_expression!(quad.aff, new_coef, new_var)
return quad
end

function add_to_expression!(
quad::GenericQuadExpr,
new_coef::_Constant,
Expand Down Expand Up @@ -391,6 +408,14 @@ function add_to_expression!(
return add_to_expression!(quad, var, aff)
end

function add_to_expression!(
quad::GenericQuadExpr{C,V},
aff::GenericAffExpr{C,V},
var::V,
) where {C,V<:Union{Number,LinearAlgebra.UniformScaling}}
return add_to_expression!(quad, var, aff)
end

function add_to_expression!(
quad::GenericQuadExpr{C,V},
lhs::GenericAffExpr{C,V},
Expand Down
15 changes: 15 additions & 0 deletions test/expr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -411,3 +411,18 @@ end
map_coefficients_inplace!(c -> 2c, y)
@test y == 2 * x[1] + 4 * x[2] + 6 * x[1]^2 + 8 * x[2]^2
end

@testset "expression_ambiguities" begin
# These tests use expressions with unusual key types so that we can test
# the fallback methods needed to avoid method ambiguities.
model = Model()
quad = GenericQuadExpr{Int,Int}()
aff = GenericAffExpr{Int,Int}(0, 1 => 1)
@test add_to_expression!(quad, aff, 1) isa GenericQuadExpr
@test quad == GenericQuadExpr{Int,Int}(aff)
quad = GenericQuadExpr{Int,Int}()
@test add_to_expression!(quad, 0, 0) isa GenericQuadExpr
@variable(model, x)
@test add_to_expression!(x + 1.0, 1.0, 2) isa GenericAffExpr
@test add_to_expression!(x^2, 1.0, 2) isa GenericQuadExpr
end
8 changes: 8 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@
#############################################################################

using Test
import JuMP

@testset "Ambiguities" begin
# It is important to run these first, before the tests start adding methods.
@test isempty(Test.detect_ambiguities(JuMP))
# TODO(odow): there are still some ambiguities in Containers
# @test isempty(Test.detect_ambiguities(JuMP.Containers))
end

t = time()
include("Containers/Containers.jl")
Expand Down

0 comments on commit f5749e5

Please sign in to comment.