From 040215d93c2115d6303db1997f14facf664aea9d Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 30 Oct 2023 10:42:55 +1300 Subject: [PATCH 1/7] Add start_value, lower_bound, and upper_bound support for GenericAffExpr --- src/aff_expr.jl | 97 +++++++++++++++++++++++++++++++++++++++++++++++ test/test_expr.jl | 89 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+) diff --git a/src/aff_expr.jl b/src/aff_expr.jl index f6b3db65361..8ac91d69933 100644 --- a/src/aff_expr.jl +++ b/src/aff_expr.jl @@ -809,3 +809,100 @@ moi_function(a::Vector{<:GenericAffExpr}) = MOI.VectorAffineFunction(a) function moi_function_type(::Type{<:Vector{<:GenericAffExpr{T}}}) where {T} return MOI.VectorAffineFunction{T} end + +# start_value(::GenericAffExpr) + +function start_value(x::GenericAffExpr) + # Unset start values return `nothing`, but we cannot do arithmetic on + # `nothing`. So convert any nothings to `missing`, then compute the `value` + # and then re-convert back to `nothing` if the result is `missing`. + to_missing_start(x) = something(start_value(x), missing) + return coalesce(value(to_missing_start, x), nothing) +end + +function set_start_value(x::GenericAffExpr, ::Nothing) + set_start_value.(keys(x.terms), nothing) + return +end + +function set_start_value(x::GenericAffExpr, value::Number) + if isempty(x.terms) && iszero(value) + return + elseif length(x.terms) != 1 + error( + "Cannot set the start value of $x because it does not contain " * + "exactly one variable", + ) + end + # a * x + b == value --> x == (value - b) / a + new_value = (value - x.constant) / first(values(x.terms)) + set_start_value(first(keys(x.terms)), new_value) + return +end + +function set_start_value(x::GenericAffExpr{T}, value::Number) where {T<:Complex} + value_T = convert(T, value) + set_start_value(real(x), real(value_T)) + set_start_value(imag(x), imag(value_T)) + return +end + +# lower_bound(::GenericAffExpr) + +has_lower_bound(x::GenericAffExpr) = all(has_lower_bound, keys(x.terms)) + +lower_bound(x::GenericAffExpr) = value(lower_bound, x) + +delete_lower_bound(x::GenericAffExpr) = delete_lower_bound.(keys(x.terms)) + +function set_lower_bound(x::GenericAffExpr, value::Number) + if isempty(x.terms) && iszero(value) + return + elseif length(x.terms) != 1 + error( + "Cannot set the lower bound of $x because it does not contain " * + "exactly one variable", + ) + end + # a * x + b >= value --> x >= (value - b) / a + new_value = (value - x.constant) / first(values(x.terms)) + set_lower_bound(first(keys(x.terms)), new_value) + return +end + +function set_lower_bound(x::GenericAffExpr{T}, value::Number) where {T<:Complex} + value_T = convert(T, value) + set_lower_bound(real(x), real(value_T)) + set_lower_bound(imag(x), imag(value_T)) + return +end + +# upper_bound(::GenericAffExpr) + +has_upper_bound(x::GenericAffExpr) = all(has_upper_bound, keys(x.terms)) + +upper_bound(x::GenericAffExpr) = value(upper_bound, x) + +delete_upper_bound(x::GenericAffExpr) = delete_upper_bound.(keys(x.terms)) + +function set_upper_bound(x::GenericAffExpr, value::Number) + if isempty(x.terms) && iszero(value) + return + elseif length(x.terms) != 1 + error( + "Cannot set the upper bound of $x because it does not contain " * + "exactly one variable", + ) + end + # a * x + b <= value --> x <= (value - b) / a + new_value = (value - x.constant) / first(values(x.terms)) + set_upper_bound(first(keys(x.terms)), new_value) + return +end + +function set_upper_bound(x::GenericAffExpr{T}, value::Number) where {T<:Complex} + value_T = convert(T, value) + set_upper_bound(real(x), real(value_T)) + set_upper_bound(imag(x), imag(value_T)) + return +end diff --git a/test/test_expr.jl b/test/test_expr.jl index 1aad5b4fb0c..01e02c4665c 100644 --- a/test/test_expr.jl +++ b/test/test_expr.jl @@ -451,4 +451,93 @@ function test_quadexpr_owner_model() return end +function test_aff_expr_complex_lower_bound() + model = Model() + @variable(model, x in ComplexPlane()) + @test !has_lower_bound(x) + set_lower_bound(x, 1 + 2im) + @test has_lower_bound(x) + @test lower_bound(x) == 1 + 2im + return +end + +function test_aff_expr_complex_upper_bound() + model = Model() + @variable(model, x in ComplexPlane()) + @test !has_upper_bound(x) + set_upper_bound(x, 3 + 4im) + @test has_upper_bound(x) + @test upper_bound(x) == 3 + 4im + return +end + +function test_aff_expr_complex_start_value() + model = Model() + @variable(model, x in ComplexPlane()) + @test start_value(x) === nothing + set_start_value(x, 2 + 3im) + @test start_value(x) == 2 + 3im + return +end + +function test_aff_expr_complex_hermitian_lower_bound() + model = Model() + @variable(model, x[1:2, 1:2] in HermitianPSDCone()) + @test !any(has_lower_bound.(x)) + A = [1 (2 + 3im); (2 - 3im) 4] + set_lower_bound.(x, A) + @test all(has_lower_bound.(x)) + @test lower_bound.(x) == A + return +end + +function test_aff_expr_complex_hermitian_upper_bound() + model = Model() + @variable(model, x[1:2, 1:2] in HermitianPSDCone()) + @test !any(has_upper_bound.(x)) + A = [1 (2 + 3im); (2 - 3im) 4] + set_upper_bound.(x, A) + @test all(has_upper_bound.(x)) + @test upper_bound.(x) == A + return +end + +function test_aff_expr_complex_hermitian_start_value() + model = Model() + @variable(model, x[1:2, 1:2] in HermitianPSDCone()) + @test !any(has_upper_bound.(x)) + A = [1 (2 + 3im); (2 - 3im) 4] + @test all(start_value.(x) .=== nothing) + set_start_value.(x, A) + @test start_value.(x) == A + return +end + +function test_aff_expr_complex_errors() + model = Model() + @variable(model, x[1:2, 1:2] in HermitianPSDCone()) + @test_throws( + ErrorException( + "Cannot set the lower bound of 0 because it does not contain " * + "exactly one variable", + ), + set_lower_bound(x[1, 1], 1 + 2im), + ) + @test_throws( + ErrorException( + "Cannot set the upper bound of 0 because it does not contain " * + "exactly one variable", + ), + set_upper_bound(x[1, 1], 1 + 2im), + ) + @test_throws( + ErrorException( + "Cannot set the start value of 0 because it does not contain " * + "exactly one variable", + ), + set_start_value(x[1, 1], 1 + 2im), + ) + return +end + end # TestExpr From f84be58afd5f3f3bf320ebdcd9ff4f7cdc2b151e Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 30 Oct 2023 10:54:34 +1300 Subject: [PATCH 2/7] Fix formatting --- test/test_expr.jl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_expr.jl b/test/test_expr.jl index 01e02c4665c..4b809c9fa70 100644 --- a/test/test_expr.jl +++ b/test/test_expr.jl @@ -484,7 +484,7 @@ function test_aff_expr_complex_hermitian_lower_bound() model = Model() @variable(model, x[1:2, 1:2] in HermitianPSDCone()) @test !any(has_lower_bound.(x)) - A = [1 (2 + 3im); (2 - 3im) 4] + A = [1 (2+3im); (2-3im) 4] set_lower_bound.(x, A) @test all(has_lower_bound.(x)) @test lower_bound.(x) == A @@ -495,7 +495,7 @@ function test_aff_expr_complex_hermitian_upper_bound() model = Model() @variable(model, x[1:2, 1:2] in HermitianPSDCone()) @test !any(has_upper_bound.(x)) - A = [1 (2 + 3im); (2 - 3im) 4] + A = [1 (2+3im); (2-3im) 4] set_upper_bound.(x, A) @test all(has_upper_bound.(x)) @test upper_bound.(x) == A @@ -506,7 +506,7 @@ function test_aff_expr_complex_hermitian_start_value() model = Model() @variable(model, x[1:2, 1:2] in HermitianPSDCone()) @test !any(has_upper_bound.(x)) - A = [1 (2 + 3im); (2 - 3im) 4] + A = [1 (2+3im); (2-3im) 4] @test all(start_value.(x) .=== nothing) set_start_value.(x, A) @test start_value.(x) == A From 6277c472f9ad798e0be6d7a2bc8990aabaec444e Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 30 Oct 2023 11:49:59 +1300 Subject: [PATCH 3/7] Various fixes --- src/aff_expr.jl | 130 ++++++++++++++++++++++++++++++++++++++++------ test/test_expr.jl | 24 +++++++++ 2 files changed, 138 insertions(+), 16 deletions(-) diff --git a/src/aff_expr.jl b/src/aff_expr.jl index 8ac91d69933..a7ef749784c 100644 --- a/src/aff_expr.jl +++ b/src/aff_expr.jl @@ -826,7 +826,7 @@ function set_start_value(x::GenericAffExpr, ::Nothing) end function set_start_value(x::GenericAffExpr, value::Number) - if isempty(x.terms) && iszero(value) + if isempty(x.terms) && x.constant == value return elseif length(x.terms) != 1 error( @@ -835,8 +835,14 @@ function set_start_value(x::GenericAffExpr, value::Number) ) end # a * x + b == value --> x == (value - b) / a - new_value = (value - x.constant) / first(values(x.terms)) - set_start_value(first(keys(x.terms)), new_value) + a = first(values(x.terms)) + if !isone(abs(a)) + error( + "Cannot set the start value of `$x` because it does not contain " * + "exactly one variable with a coefficient of `+1` or `-1`", + ) + end + set_start_value(first(keys(x.terms)), (value - x.constant) / a) return end @@ -849,14 +855,51 @@ end # lower_bound(::GenericAffExpr) -has_lower_bound(x::GenericAffExpr) = all(has_lower_bound, keys(x.terms)) +function has_lower_bound(x::GenericAffExpr) + return all(x.terms) do (k, v) + iszero(v) || (v > 0 && has_lower_bound(k)) || (v < 0 && has_upper_bound(k)) + end +end + +function has_lower_bound(x::GenericAffExpr{<:Complex}) + return has_lower_bound(real(x)) && has_lower_bound(imag(x)) +end + +function lower_bound(x::GenericAffExpr{T}) where {T} + y = x.constant + for (k, v) in x.terms + if v > 0 + y += v * lower_bound(k) + elseif v < 0 + y += v * upper_bound(k) + end + end + return y +end + +function lower_bound(x::GenericAffExpr{<:Complex}) + return lower_bound(real(x)) + lower_bound(imag(x)) * im +end -lower_bound(x::GenericAffExpr) = value(lower_bound, x) +function delete_lower_bound(x::GenericAffExpr{T}) where {T} + for (k, v) in x.terms + if v > 0 && has_lower_bound(k) + delete_lower_bound(k) + elseif v < 0 && has_upper_bound(k) + delete_upper_bound(k) + end + end + return +end -delete_lower_bound(x::GenericAffExpr) = delete_lower_bound.(keys(x.terms)) +function delete_lower_bound(x::GenericAffExpr{<:Complex}) + delete_lower_bound(real(x)) + delete_lower_bound(imag(x)) + return +end function set_lower_bound(x::GenericAffExpr, value::Number) - if isempty(x.terms) && iszero(value) + if isempty(x.terms) && x.constant == value return elseif length(x.terms) != 1 error( @@ -865,8 +908,17 @@ function set_lower_bound(x::GenericAffExpr, value::Number) ) end # a * x + b >= value --> x >= (value - b) / a - new_value = (value - x.constant) / first(values(x.terms)) - set_lower_bound(first(keys(x.terms)), new_value) + a = first(values(x.terms)) + if isone(a) + set_lower_bound(first(keys(x.terms)), value - x.constant) + elseif isone(-a) + set_upper_bound(first(keys(x.terms)), x.constant - value) + else + error( + "Cannot set the lower bound of `$x` because it does not contain " * + "exactly one variable with a coefficient of `+1` or `-1`", + ) + end return end @@ -879,14 +931,51 @@ end # upper_bound(::GenericAffExpr) -has_upper_bound(x::GenericAffExpr) = all(has_upper_bound, keys(x.terms)) +function has_upper_bound(x::GenericAffExpr) + return all(x.terms) do (k, v) + (v > 0 && has_upper_bound(k)) || (v < 0 && has_lower_bound(k)) + end +end + +function has_upper_bound(x::GenericAffExpr{<:Complex}) + return has_upper_bound(real(x)) && has_upper_bound(imag(x)) +end + +function upper_bound(x::GenericAffExpr{T}) where {T} + y = x.constant + for (k, v) in x.terms + if v > 0 + y += v * upper_bound(k) + elseif v < 0 + y += v * lower_bound(k) + end + end + return y +end + +function upper_bound(x::GenericAffExpr{<:Complex}) + return upper_bound(real(x)) + upper_bound(imag(x)) * im +end -upper_bound(x::GenericAffExpr) = value(upper_bound, x) +function delete_upper_bound(x::GenericAffExpr{T}) where {T} + for (k, v) in x.terms + if v > 0 && has_upper_bound(k) + delete_upper_bound(k) + elseif v < 0 && has_lower_bound(k) + delete_lower_bound(k) + end + end + return +end -delete_upper_bound(x::GenericAffExpr) = delete_upper_bound.(keys(x.terms)) +function delete_upper_bound(x::GenericAffExpr{<:Complex}) + delete_upper_bound(real(x)) + delete_upper_bound(imag(x)) + return +end function set_upper_bound(x::GenericAffExpr, value::Number) - if isempty(x.terms) && iszero(value) + if isempty(x.terms) && x.constant == value return elseif length(x.terms) != 1 error( @@ -894,9 +983,18 @@ function set_upper_bound(x::GenericAffExpr, value::Number) "exactly one variable", ) end - # a * x + b <= value --> x <= (value - b) / a - new_value = (value - x.constant) / first(values(x.terms)) - set_upper_bound(first(keys(x.terms)), new_value) + # a * x + b >= value --> x >= (value - b) / a + a = first(values(x.terms)) + if isone(a) + set_upper_bound(first(keys(x.terms)), value - x.constant) + elseif isone(-a) + set_lower_bound(first(keys(x.terms)), x.constant - value) + else + error( + "Cannot set the upper bound of `$x` because it does not contain " * + "exactly one variable with a coefficient of `+1` or `-1`", + ) + end return end diff --git a/test/test_expr.jl b/test/test_expr.jl index 4b809c9fa70..0131207c0c2 100644 --- a/test/test_expr.jl +++ b/test/test_expr.jl @@ -461,6 +461,15 @@ function test_aff_expr_complex_lower_bound() return end +function test_aff_expr_complex_lower_bound_negative() + model = Model() + @variable(model, -3 <= x <= 4) + y = (1 - 2im) * x + @test has_lower_bound(y) + @test lower_bound(y) == -3.0 - 8im + return +end + function test_aff_expr_complex_upper_bound() model = Model() @variable(model, x in ComplexPlane()) @@ -537,6 +546,21 @@ function test_aff_expr_complex_errors() ), set_start_value(x[1, 1], 1 + 2im), ) + y = 2 * x[1, 1] + @test_throws( + ErrorException( + "Cannot set the lower bound of `$y` because it does not contain " * + "exactly one variable with a coefficient of `+1` or `-1`", + ), + set_lower_bound(y, 1 + 2im), + ) + @test_throws( + ErrorException( + "Cannot set the upper bound of `$y` because it does not contain " * + "exactly one variable with a coefficient of `+1` or `-1`", + ), + set_upper_bound(y, 1 + 2im), + ) return end From 5798da98201d7a263e1076c5cff0ece0f1bba872 Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 30 Oct 2023 12:27:09 +1300 Subject: [PATCH 4/7] Simplify cases that we support --- src/aff_expr.jl | 210 ++++++++++------------------------------------ test/test_expr.jl | 107 +++++++---------------- 2 files changed, 72 insertions(+), 245 deletions(-) diff --git a/src/aff_expr.jl b/src/aff_expr.jl index a7ef749784c..636e3a70209 100644 --- a/src/aff_expr.jl +++ b/src/aff_expr.jl @@ -810,197 +810,71 @@ function moi_function_type(::Type{<:Vector{<:GenericAffExpr{T}}}) where {T} return MOI.VectorAffineFunction{T} end -# start_value(::GenericAffExpr) - -function start_value(x::GenericAffExpr) - # Unset start values return `nothing`, but we cannot do arithmetic on - # `nothing`. So convert any nothings to `missing`, then compute the `value` - # and then re-convert back to `nothing` if the result is `missing`. - to_missing_start(x) = something(start_value(x), missing) - return coalesce(value(to_missing_start, x), nothing) -end - -function set_start_value(x::GenericAffExpr, ::Nothing) - set_start_value.(keys(x.terms), nothing) - return -end +""" + _eval_as_variable(f::F, x::GenericAffExpr, args...) where {F} -function set_start_value(x::GenericAffExpr, value::Number) - if isempty(x.terms) && x.constant == value - return - elseif length(x.terms) != 1 +In many cases, `@variable` can return a `GenericAffExpr` instead of a +`GenericVariableRef`. This is particularly the case for complex-valued +expressions. To make common operatons like `lower_bound(x)` work, we should +forward the method if and only if `x` is convertable to a `GenericVariableRef`. +""" +function _eval_as_variable(f::F, x::GenericAffExpr, args...) where {F} + if length(x.terms) != 1 error( - "Cannot set the start value of $x because it does not contain " * - "exactly one variable", + "Cannot call $f with $x because it is not a real-valued affine " * + "expression of one variable.", ) - end - # a * x + b == value --> x == (value - b) / a - a = first(values(x.terms)) - if !isone(abs(a)) + elseif !isone(first(keys(x.terms))) error( - "Cannot set the start value of `$x` because it does not contain " * - "exactly one variable with a coefficient of `+1` or `-1`", + "Cannot call $f with $x because it is not a real-valued affine " * + "expression of one variable with a coefficient of `+1`.", ) end - set_start_value(first(keys(x.terms)), (value - x.constant) / a) - return -end - -function set_start_value(x::GenericAffExpr{T}, value::Number) where {T<:Complex} - value_T = convert(T, value) - set_start_value(real(x), real(value_T)) - set_start_value(imag(x), imag(value_T)) - return + return f(first(keys(x.terms)), args...) end -# lower_bound(::GenericAffExpr) - -function has_lower_bound(x::GenericAffExpr) - return all(x.terms) do (k, v) - iszero(v) || (v > 0 && has_lower_bound(k)) || (v < 0 && has_upper_bound(k)) - end -end - -function has_lower_bound(x::GenericAffExpr{<:Complex}) - return has_lower_bound(real(x)) && has_lower_bound(imag(x)) -end - -function lower_bound(x::GenericAffExpr{T}) where {T} - y = x.constant - for (k, v) in x.terms - if v > 0 - y += v * lower_bound(k) - elseif v < 0 - y += v * upper_bound(k) - end - end - return y -end - -function lower_bound(x::GenericAffExpr{<:Complex}) - return lower_bound(real(x)) + lower_bound(imag(x)) * im +function _eval_as_variable( + f::F, + x::GenericAffExpr{T}, + args..., +) where {F,T<:Complex} + return error( + "Cannot call $f with $x because it is not a real-valued affine " * + "expression of one variable with a coefficient of `+1`. Use " * + "`real(x)` or `imag(x)` to obtain the real and imaginary part and " * + "pass that instead.", + ) end -function delete_lower_bound(x::GenericAffExpr{T}) where {T} - for (k, v) in x.terms - if v > 0 && has_lower_bound(k) - delete_lower_bound(k) - elseif v < 0 && has_upper_bound(k) - delete_upper_bound(k) - end - end - return -end +# start_value(::GenericAffExpr) -function delete_lower_bound(x::GenericAffExpr{<:Complex}) - delete_lower_bound(real(x)) - delete_lower_bound(imag(x)) - return -end +start_value(x::GenericAffExpr) = _eval_as_variable(start_value, x) -function set_lower_bound(x::GenericAffExpr, value::Number) - if isempty(x.terms) && x.constant == value - return - elseif length(x.terms) != 1 - error( - "Cannot set the lower bound of $x because it does not contain " * - "exactly one variable", - ) - end - # a * x + b >= value --> x >= (value - b) / a - a = first(values(x.terms)) - if isone(a) - set_lower_bound(first(keys(x.terms)), value - x.constant) - elseif isone(-a) - set_upper_bound(first(keys(x.terms)), x.constant - value) - else - error( - "Cannot set the lower bound of `$x` because it does not contain " * - "exactly one variable with a coefficient of `+1` or `-1`", - ) - end +function set_start_value(x::GenericAffExpr, value) + _eval_as_variable(set_start_value, x, value) return end -function set_lower_bound(x::GenericAffExpr{T}, value::Number) where {T<:Complex} - value_T = convert(T, value) - set_lower_bound(real(x), real(value_T)) - set_lower_bound(imag(x), imag(value_T)) - return -end +# lower_bound(::GenericAffExpr) -# upper_bound(::GenericAffExpr) +has_lower_bound(x::GenericAffExpr) = _eval_as_variable(has_lower_bound, x) -function has_upper_bound(x::GenericAffExpr) - return all(x.terms) do (k, v) - (v > 0 && has_upper_bound(k)) || (v < 0 && has_lower_bound(k)) - end -end +lower_bound(x::GenericAffExpr) = _eval_as_variable(lower_bound, x) -function has_upper_bound(x::GenericAffExpr{<:Complex}) - return has_upper_bound(real(x)) && has_upper_bound(imag(x)) -end +delete_lower_bound(x::GenericAffExpr) = _eval_as_variable(delete_lower_bound, x) -function upper_bound(x::GenericAffExpr{T}) where {T} - y = x.constant - for (k, v) in x.terms - if v > 0 - y += v * upper_bound(k) - elseif v < 0 - y += v * lower_bound(k) - end - end - return y +function set_lower_bound(x::GenericAffExpr, value) + return _eval_as_variable(set_lower_bound, x, value) end -function upper_bound(x::GenericAffExpr{<:Complex}) - return upper_bound(real(x)) + upper_bound(imag(x)) * im -end +# upper_bound(::GenericAffExpr) -function delete_upper_bound(x::GenericAffExpr{T}) where {T} - for (k, v) in x.terms - if v > 0 && has_upper_bound(k) - delete_upper_bound(k) - elseif v < 0 && has_lower_bound(k) - delete_lower_bound(k) - end - end - return -end +has_upper_bound(x::GenericAffExpr) = _eval_as_variable(has_upper_bound, x) -function delete_upper_bound(x::GenericAffExpr{<:Complex}) - delete_upper_bound(real(x)) - delete_upper_bound(imag(x)) - return -end +upper_bound(x::GenericAffExpr) = _eval_as_variable(upper_bound, x) -function set_upper_bound(x::GenericAffExpr, value::Number) - if isempty(x.terms) && x.constant == value - return - elseif length(x.terms) != 1 - error( - "Cannot set the upper bound of $x because it does not contain " * - "exactly one variable", - ) - end - # a * x + b >= value --> x >= (value - b) / a - a = first(values(x.terms)) - if isone(a) - set_upper_bound(first(keys(x.terms)), value - x.constant) - elseif isone(-a) - set_lower_bound(first(keys(x.terms)), x.constant - value) - else - error( - "Cannot set the upper bound of `$x` because it does not contain " * - "exactly one variable with a coefficient of `+1` or `-1`", - ) - end - return -end +delete_upper_bound(x::GenericAffExpr) = _eval_as_variable(delete_upper_bound, x) -function set_upper_bound(x::GenericAffExpr{T}, value::Number) where {T<:Complex} - value_T = convert(T, value) - set_upper_bound(real(x), real(value_T)) - set_upper_bound(imag(x), imag(value_T)) - return +function set_upper_bound(x::GenericAffExpr, value) + return _eval_as_variable(set_upper_bound, x, value) end diff --git a/test/test_expr.jl b/test/test_expr.jl index 0131207c0c2..90905fbbf49 100644 --- a/test/test_expr.jl +++ b/test/test_expr.jl @@ -454,112 +454,65 @@ end function test_aff_expr_complex_lower_bound() model = Model() @variable(model, x in ComplexPlane()) - @test !has_lower_bound(x) - set_lower_bound(x, 1 + 2im) - @test has_lower_bound(x) - @test lower_bound(x) == 1 + 2im - return -end - -function test_aff_expr_complex_lower_bound_negative() - model = Model() - @variable(model, -3 <= x <= 4) - y = (1 - 2im) * x + y = real(x) + @test !has_lower_bound(y) + set_lower_bound(y, 1) @test has_lower_bound(y) - @test lower_bound(y) == -3.0 - 8im + @test lower_bound(y) == 1 + delete_lower_bound(y) + @test !has_lower_bound(y) return end function test_aff_expr_complex_upper_bound() model = Model() @variable(model, x in ComplexPlane()) - @test !has_upper_bound(x) - set_upper_bound(x, 3 + 4im) - @test has_upper_bound(x) - @test upper_bound(x) == 3 + 4im + y = real(x) + @test !has_upper_bound(y) + set_upper_bound(y, 1) + @test has_upper_bound(y) + @test upper_bound(y) == 1 + delete_upper_bound(y) + @test !has_upper_bound(y) return end function test_aff_expr_complex_start_value() model = Model() @variable(model, x in ComplexPlane()) - @test start_value(x) === nothing - set_start_value(x, 2 + 3im) - @test start_value(x) == 2 + 3im - return -end - -function test_aff_expr_complex_hermitian_lower_bound() - model = Model() - @variable(model, x[1:2, 1:2] in HermitianPSDCone()) - @test !any(has_lower_bound.(x)) - A = [1 (2+3im); (2-3im) 4] - set_lower_bound.(x, A) - @test all(has_lower_bound.(x)) - @test lower_bound.(x) == A - return -end - -function test_aff_expr_complex_hermitian_upper_bound() - model = Model() - @variable(model, x[1:2, 1:2] in HermitianPSDCone()) - @test !any(has_upper_bound.(x)) - A = [1 (2+3im); (2-3im) 4] - set_upper_bound.(x, A) - @test all(has_upper_bound.(x)) - @test upper_bound.(x) == A + y = real(x) + @test start_value(y) === nothing + set_start_value(y, 1) + @test start_value(y) == 1 return end -function test_aff_expr_complex_hermitian_start_value() +function test_aff_expr_complex_error() model = Model() @variable(model, x[1:2, 1:2] in HermitianPSDCone()) - @test !any(has_upper_bound.(x)) - A = [1 (2+3im); (2-3im) 4] - @test all(start_value.(x) .=== nothing) - set_start_value.(x, A) - @test start_value.(x) == A - return -end - -function test_aff_expr_complex_errors() - model = Model() - @variable(model, x[1:2, 1:2] in HermitianPSDCone()) - @test_throws( - ErrorException( - "Cannot set the lower bound of 0 because it does not contain " * - "exactly one variable", - ), - set_lower_bound(x[1, 1], 1 + 2im), - ) - @test_throws( - ErrorException( - "Cannot set the upper bound of 0 because it does not contain " * - "exactly one variable", - ), - set_upper_bound(x[1, 1], 1 + 2im), - ) @test_throws( ErrorException( - "Cannot set the start value of 0 because it does not contain " * - "exactly one variable", + "Cannot call $start_value with $(x[1, 1]) because it is not a real-valued affine " * + "expression of one variable with a coefficient of `+1`. Use " * + "`real(x)` or `imag(x)` to obtain the real and imaginary part and " * + "pass that instead.", ), - set_start_value(x[1, 1], 1 + 2im), + start_value(x[1, 1]), ) - y = 2 * x[1, 1] @test_throws( ErrorException( - "Cannot set the lower bound of `$y` because it does not contain " * - "exactly one variable with a coefficient of `+1` or `-1`", + "Cannot call $start_value with $(imag(x[2, 1])) because it is not a real-valued affine " * + "expression of one variable with a coefficient of `+1`.", ), - set_lower_bound(y, 1 + 2im), + start_value(imag(x[2, 1])), ) + y = AffExpr(0.0) @test_throws( ErrorException( - "Cannot set the upper bound of `$y` because it does not contain " * - "exactly one variable with a coefficient of `+1` or `-1`", + "Cannot call $start_value with $y because it is not a real-valued affine " * + "expression of one variable.", ), - set_upper_bound(y, 1 + 2im), + start_value(y), ) return end From 2fe0a5aa7a90c121d708af24fd4ef7a20aec2948 Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 30 Oct 2023 12:38:03 +1300 Subject: [PATCH 5/7] Fix --- src/aff_expr.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/aff_expr.jl b/src/aff_expr.jl index 636e3a70209..cfb54aaef51 100644 --- a/src/aff_expr.jl +++ b/src/aff_expr.jl @@ -824,7 +824,7 @@ function _eval_as_variable(f::F, x::GenericAffExpr, args...) where {F} "Cannot call $f with $x because it is not a real-valued affine " * "expression of one variable.", ) - elseif !isone(first(keys(x.terms))) + elseif !isone(first(values(x.terms))) error( "Cannot call $f with $x because it is not a real-valued affine " * "expression of one variable with a coefficient of `+1`.", From 88740928a3cef626feacf0417366323ae3026890 Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 30 Oct 2023 13:08:01 +1300 Subject: [PATCH 6/7] Fix docs --- docs/src/manual/variables.md | 19 ------------------- src/aff_expr.jl | 6 ++++-- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/docs/src/manual/variables.md b/docs/src/manual/variables.md index aa69b829841..ce8d6cf11a5 100644 --- a/docs/src/manual/variables.md +++ b/docs/src/manual/variables.md @@ -1214,25 +1214,6 @@ julia> @variable(model, x[1:2, 1:2] in SkewSymmetricMatrixSpace()) `model`; the remaining elements in `x` are linear transformations of the single variable. -Because the returned matrix `x` is `Matrix{AffExpr}`, you cannot use -variable-related functions on its elements: -```jldoctest skewsymmetric -julia> set_lower_bound(x[1, 2], 0.0) -ERROR: MethodError: no method matching set_lower_bound(::AffExpr, ::Float64) -[...] -``` - -Instead, you can convert an upper-triangular elements to a variable as follows: -```jldoctest skewsymmetric -julia> to_variable(x::AffExpr) = first(keys(x.terms)) -to_variable (generic function with 1 method) - -julia> to_variable(x[1, 2]) -x[1,2] - -julia> set_lower_bound(to_variable(x[1, 2]), 0.0) -``` - ### Example: Hermitian positive semidefinite variables Declare a matrix of JuMP variables to be Hermitian positive semidefinite using diff --git a/src/aff_expr.jl b/src/aff_expr.jl index cfb54aaef51..faa7a5c0f15 100644 --- a/src/aff_expr.jl +++ b/src/aff_expr.jl @@ -824,13 +824,15 @@ function _eval_as_variable(f::F, x::GenericAffExpr, args...) where {F} "Cannot call $f with $x because it is not a real-valued affine " * "expression of one variable.", ) - elseif !isone(first(values(x.terms))) + end + variable, coefficient = first(x.terms) + if !isone(coefficient) error( "Cannot call $f with $x because it is not a real-valued affine " * "expression of one variable with a coefficient of `+1`.", ) end - return f(first(keys(x.terms)), args...) + return f(variable, args...) end function _eval_as_variable( From 4e256eff6e1dbb0913627cd62abf9d8fde92cac2 Mon Sep 17 00:00:00 2001 From: odow Date: Tue, 31 Oct 2023 11:17:59 +1300 Subject: [PATCH 7/7] Updates --- src/aff_expr.jl | 21 ++++----------------- test/test_expr.jl | 20 +++++++++++--------- 2 files changed, 15 insertions(+), 26 deletions(-) diff --git a/src/aff_expr.jl b/src/aff_expr.jl index faa7a5c0f15..86570a9ad2b 100644 --- a/src/aff_expr.jl +++ b/src/aff_expr.jl @@ -821,33 +821,20 @@ forward the method if and only if `x` is convertable to a `GenericVariableRef`. function _eval_as_variable(f::F, x::GenericAffExpr, args...) where {F} if length(x.terms) != 1 error( - "Cannot call $f with $x because it is not a real-valued affine " * - "expression of one variable.", + "Cannot call $f with $x because it is not an affine expression " * + "of one variable.", ) end variable, coefficient = first(x.terms) if !isone(coefficient) error( - "Cannot call $f with $x because it is not a real-valued affine " * - "expression of one variable with a coefficient of `+1`.", + "Cannot call $f with $x because the variable has a coefficient " * + "that is different to `+1`.", ) end return f(variable, args...) end -function _eval_as_variable( - f::F, - x::GenericAffExpr{T}, - args..., -) where {F,T<:Complex} - return error( - "Cannot call $f with $x because it is not a real-valued affine " * - "expression of one variable with a coefficient of `+1`. Use " * - "`real(x)` or `imag(x)` to obtain the real and imaginary part and " * - "pass that instead.", - ) -end - # start_value(::GenericAffExpr) start_value(x::GenericAffExpr) = _eval_as_variable(start_value, x) diff --git a/test/test_expr.jl b/test/test_expr.jl index 90905fbbf49..1a446493496 100644 --- a/test/test_expr.jl +++ b/test/test_expr.jl @@ -487,29 +487,31 @@ function test_aff_expr_complex_start_value() return end -function test_aff_expr_complex_error() +function test_aff_expr_complex_HermitianPSDCone() model = Model() @variable(model, x[1:2, 1:2] in HermitianPSDCone()) + @test start_value(x[1, 1]) === nothing + set_lower_bound(x[1, 1], 2.5) + @test has_lower_bound(x[1, 1]) + @test lower_bound(x[1, 1]) == 2.5 @test_throws( ErrorException( - "Cannot call $start_value with $(x[1, 1]) because it is not a real-valued affine " * - "expression of one variable with a coefficient of `+1`. Use " * - "`real(x)` or `imag(x)` to obtain the real and imaginary part and " * - "pass that instead.", + "Cannot call $start_value with $(x[2, 1]) because it is not an affine " * + "expression of one variable.", ), - start_value(x[1, 1]), + start_value(x[2, 1]), ) @test_throws( ErrorException( - "Cannot call $start_value with $(imag(x[2, 1])) because it is not a real-valued affine " * - "expression of one variable with a coefficient of `+1`.", + "Cannot call $start_value with $(imag(x[2, 1])) because the " * + "variable has a coefficient that is different to `+1`.", ), start_value(imag(x[2, 1])), ) y = AffExpr(0.0) @test_throws( ErrorException( - "Cannot call $start_value with $y because it is not a real-valued affine " * + "Cannot call $start_value with $y because it is not an affine " * "expression of one variable.", ), start_value(y),