Skip to content

Commit

Permalink
Merge pull request jump-dev#2178 from JuliaOpt/bl/scalar_product
Browse files Browse the repository at this point in the history
Fix sum of expr with scalar product
  • Loading branch information
blegat authored Feb 25, 2020
2 parents 13ad5db + 77a73dc commit fa302b5
Showing 1 changed file with 13 additions and 6 deletions.
19 changes: 13 additions & 6 deletions src/mutable_arithmetics.jl
Original file line number Diff line number Diff line change
Expand Up @@ -128,24 +128,33 @@ function _MA.mutable_operate!(::typeof(-), expr::_GenericAffOrQuadExpr, x)
return add_to_expression!(expr, -1, x)
end

const _Scalar = Union{AbstractJuMPScalar, _Constant}

# `add_to_expression!` is responsible to implement all methods of up to 3 arguments.
# in addition to `add_to_expression(::GenericQuadExpr, ::Real, ::AbstractVariableRef, ::AbstractVariableRef)`.
function _MA.mutable_operate!(::typeof(_MA.add_mul), expr::_GenericAffOrQuadExpr, x)
function _MA.mutable_operate!(::typeof(_MA.add_mul), expr::_GenericAffOrQuadExpr, x::_Scalar)
return add_to_expression!(expr, x)
end
function _MA.mutable_operate!(::typeof(_MA.add_mul), expr::_GenericAffOrQuadExpr, x, y)
function _MA.mutable_operate!(::typeof(_MA.add_mul), expr::_GenericAffOrQuadExpr, x::_Scalar, y::_Scalar)
return add_to_expression!(expr, x, y)
end
function _MA.mutable_operate!(::typeof(_MA.sub_mul), expr::_GenericAffOrQuadExpr, x)
function _MA.mutable_operate!(::typeof(_MA.sub_mul), expr::_GenericAffOrQuadExpr, x::_Scalar)
return add_to_expression!(expr, -1.0, x)
end
function _MA.mutable_operate!(::typeof(_MA.sub_mul), expr::_GenericAffOrQuadExpr, x, y)
function _MA.mutable_operate!(::typeof(_MA.sub_mul), expr::_GenericAffOrQuadExpr, x::_Scalar, y::_Scalar)
return add_to_expression!(expr, -x, y)
end
# It is less costly to negate a constant than a JuMP scalar
function _MA.mutable_operate!(::typeof(_MA.sub_mul), expr::_GenericAffOrQuadExpr, x::AbstractJuMPScalar, y::_Constant)
return add_to_expression!(expr, x, -y)
end

# If `x` could be a transposed vector and `y` a vector, they are not subtypes
# of `_Scalar` but their product is.
function _MA.mutable_operate!(op::_MA.AddSubMul, expr::_GenericAffOrQuadExpr, x, y)
return _MA.mutable_operate!(op, expr, x * y)
end

# If there are more arguments, we multiply the constants together.
@generated function _add_sub_mul_reorder!(op::_MA.AddSubMul, expr::_GenericAffOrQuadExpr, args::Vararg{Any, N}) where N
n = length(args)
Expand All @@ -170,8 +179,6 @@ end

const _AffineLike = Union{AbstractVariableRef, GenericAffExpr, _Constant}

const _Scalar = Union{AbstractJuMPScalar, _Constant}

# `add_mul(expr, args...)` defaults to `muladd(args..., expr)` which gives
# `*(args...) + expr`. If `expr isa AbstractJuMPScalar`, this reorders the terms.
# The following implementation avoids this issue and is also more efficient.
Expand Down

0 comments on commit fa302b5

Please sign in to comment.