Improve error message for non-broadcasted addition and subtraction

jump-dev · Nov 3, 2023 · a6098fd · a6098fd
1 parent 944e5c5
commit a6098fd
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 43 deletions.
diff --git a/src/operators.jl b/src/operators.jl
@@ -452,22 +452,37 @@ function LinearAlgebra.issymmetric(x::Matrix{T}) where {T<:_JuMPTypes}
     return true
 end
 
-function Base.:+(A::AbstractMatrix, x::AbstractJuMPScalar)
+function _throw_operator_error(::Union{typeof(+),typeof(_MA.add_mul)})
     return error(
-        "Addition between a Matrix and a JuMP variable is not supported: instead of `A + x`, " *
-        "do `A .+ x` for element-wise addition, or if you are modifying the diagonal entries of the matrix " *
-        "do `A + x * LinearAlgebra.I(n)`, where `n` is the diagonal length.",
+        "Addition between an array and a JuMP scalar is not supported: " *
+        "instead of `x + y`, do `x .+ y` for element-wise addition.",
     )
 end
 
-Base.:+(x::AbstractJuMPScalar, A::AbstractMatrix) = A + x
-
-function Base.:-(A::AbstractMatrix, x::AbstractJuMPScalar)
+function _throw_operator_error(::Union{typeof(-),typeof(_MA.sub_mul)})
     return error(
-        "Subtraction between a Matrix and a JuMP variable is not supported: instead of `A - x`, " *
-        "do `A .- x` for element-wise subtraction, or if you are modifying the diagonal entries of the matrix " *
-        "do `A - x * LinearAlgebra.I(n)`, where `n` is the diagonal length.",
+        "Subtraction between a Matrix and a JuMP scalar is not supported: " *
+        "instead of `x - y`, do `x .- y` for element-wise subtraction.",
     )
 end
 
-Base.:-(x::AbstractJuMPScalar, A::AbstractMatrix) = A - x
+Base.:+(::AbstractJuMPScalar, ::AbstractMatrix) = _throw_operator_error(+)
+Base.:+(::AbstractMatrix, ::AbstractJuMPScalar) = _throw_operator_error(+)
+Base.:-(::AbstractJuMPScalar, ::AbstractMatrix) = _throw_operator_error(-)
+Base.:-(::AbstractMatrix, ::AbstractJuMPScalar) = _throw_operator_error(-)
+
+function _MA.operate!!(
+    op::Union{typeof(_MA.add_mul),typeof(_MA.sub_mul)},
+    ::AbstractArray,
+    ::AbstractJuMPScalar,
+)
+    return _throw_operator_error(op)
+end
+
+function _MA.operate!!(
+    op::Union{typeof(_MA.add_mul),typeof(_MA.sub_mul)},
+    ::AbstractJuMPScalar,
+    ::AbstractArray,
+)
+    return _throw_operator_error(op)
+end
diff --git a/test/test_operator.jl b/test/test_operator.jl
@@ -629,38 +629,30 @@ function test_matrix_abstractscalar_add()
     model = Model()
     @variable(model, x)
     A = rand(Float64, 2, 2)
-    @test_throws(
-        ErrorException(
-            "Addition between a Matrix and a JuMP variable is not supported: instead of `A + x`, " *
-            "do `A .+ x` for element-wise addition, or if you are modifying the diagonal entries of the matrix " *
-            "do `A + x * LinearAlgebra.I(n)`, where `n` is the diagonal length.",
-        ),
-        A + x
-    ),
-    @test_throws(
-        ErrorException(
-            "Addition between a Matrix and a JuMP variable is not supported: instead of `A + x`, " *
-            "do `A .+ x` for element-wise addition, or if you are modifying the diagonal entries of the matrix " *
-            "do `A + x * LinearAlgebra.I(n)`, where `n` is the diagonal length.",
-        ),
-        x + A
-    ),
-    @test_throws(
-        ErrorException(
-            "Subtraction between a Matrix and a JuMP variable is not supported: instead of `A - x`, " *
-            "do `A .- x` for element-wise subtraction, or if you are modifying the diagonal entries of the matrix " *
-            "do `A - x * LinearAlgebra.I(n)`, where `n` is the diagonal length.",
-        ),
-        A - x
-    ),
-    @test_throws(
-        ErrorException(
-            "Subtraction between a Matrix and a JuMP variable is not supported: instead of `A - x`, " *
-            "do `A .- x` for element-wise subtraction, or if you are modifying the diagonal entries of the matrix " *
-            "do `A - x * LinearAlgebra.I(n)`, where `n` is the diagonal length.",
-        ),
-        x - A
-    ),
+    err_add = ErrorException(
+        "Addition between an array and a JuMP scalar is not supported: " *
+        "instead of `x + y`, do `x .+ y` for element-wise addition.",
+    )
+    @test_throws(err_add, A + x)
+    @test_throws(err_add, x + A)
+    @test_throws(err_add, A' + x)
+    @test_throws(err_add, x + A')
+    @test_throws(err_add, @expression(model, A + x))
+    @test_throws(err_add, @expression(model, x + A))
+    @test_throws(err_add, @expression(model, A' + x))
+    @test_throws(err_add, @expression(model, x + A'))
+    err_sub = ErrorException(
+        "Subtraction between a Matrix and a JuMP scalar is not supported: " *
+        "instead of `x - y`, do `x .- y` for element-wise subtraction.",
+    )
+    @test_throws(err_sub, A - x)
+    @test_throws(err_sub, x - A)
+    @test_throws(err_sub, A' - x)
+    @test_throws(err_sub, x - A')
+    @test_throws(err_sub, @expression(model, A - x))
+    @test_throws(err_sub, @expression(model, x - A))
+    @test_throws(err_sub, @expression(model, A' - x))
+    @test_throws(err_sub, @expression(model, x - A'))
     return
 end