Skip to content

Commit 1ae57de

Browse files
authored
Fix quadform with fixed variables that are modified between solves (#586)
1 parent 8272465 commit 1ae57de

File tree

3 files changed

+38
-20
lines changed

3 files changed

+38
-20
lines changed

src/problem_depot/problems/socp.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ end
376376
Hval = randn(n, n)
377377
Hval .= Hval' * Hval + 10 * diagm(0 => ones(n)) # symmetric positive definite
378378
fix!(H, Hval)
379-
p = minimize(x'b + quadform(x, H), [x >= 0]; numeric_type = T)
379+
p = minimize(x'b + quadform(x, evaluate(H)), [x >= 0]; numeric_type = T)
380380
handle_problem!(p)
381381

382382
p2 = minimize(x'b + quadform(x, Hval), [x >= 0]; numeric_type = T)

src/reformulations/quadform.jl

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,18 @@ function _is_psd(
4949
end
5050
end
5151

52+
"""
53+
quadform(x::AbstractExpr, A::AbstractExpr; assume_psd=false)
54+
55+
Represents `x' * A * x` where either:
56+
57+
* `x` is a vector-valued variable and `A` is a positive semidefinite or
58+
negative semidefinite matrix (and in particular Hermitian or real symmetric).
59+
If `assume_psd=true`, then `A` will be assumed to be positive semidefinite.
60+
Otherwise, `Convex._is_psd` will be used to check if `A` is positive
61+
semidefinite or negative semidefinite.
62+
* or `A` is a matrix-valued variable and `x` is a vector.
63+
"""
5264
quadform(x::Value, A::AbstractExpr; assume_psd = false) = x' * A * x
5365

5466
function quadform(x::AbstractExpr, A::Value; assume_psd = false)
@@ -68,24 +80,22 @@ function quadform(x::AbstractExpr, A::Value; assume_psd = false)
6880
return factor * square(norm2(P * x))
6981
end
7082

71-
"""
72-
quadform(x::AbstractExpr, A::AbstractExpr; assume_psd=false)
83+
function quadform(x::Constant, A::AbstractExpr; assume_psd = false)
84+
return quadform(evaluate(x), A; assume_psd)
85+
end
7386

74-
Represents `x' * A * x` where either:
87+
function quadform(x::AbstractExpr, A::Constant; assume_psd = false)
88+
return quadform(x, evaluate(A); assume_psd)
89+
end
7590

76-
* `x` is a vector-valued variable and `A` is a positive semidefinite or
77-
negative semidefinite matrix (and in particular Hermitian or real symmetric).
78-
If `assume_psd=true`, then `A` will be assumed to be positive semidefinite.
79-
Otherwise, `Convex._is_psd` will be used to check if `A` is positive
80-
semidefinite or negative semidefinite.
81-
* or `A` is a matrix-valued variable and `x` is a vector.
82-
"""
8391
function quadform(x::AbstractExpr, A::AbstractExpr; assume_psd = false)
84-
if vexity(x) == ConstVexity()
85-
return quadform(evaluate(x), A; assume_psd = assume_psd)
86-
elseif vexity(A) == ConstVexity()
87-
return quadform(x, evaluate(A); assume_psd = assume_psd)
88-
else
89-
error("either `x` or `A` must be constant in `quadform(x, A)`")
90-
end
92+
return error(
93+
"Convex.jl v0.13.5 introduced the ability to use `fix!`ed variables " *
94+
"in `quadform`. However, this did not consider the case that the " *
95+
"value of `fix!`ed variables is changed between solves. Due to the " *
96+
"risk that this may silently produce incorrect solutions, this " *
97+
"behavior has been removed. Use `evaluate(H)` to obtain the value of " *
98+
"a fixed variable. If the value changes between solves, rebuild the " *
99+
"problem for the change to take effect.",
100+
)
91101
end

test/test_atoms.jl

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1954,11 +1954,19 @@ function test_quadform()
19541954
_test_reformulation(target) do context
19551955
return 1 + quadform(Variable(2), constant(-[20 16; 16 20]))
19561956
end
1957+
H = Variable(2, 2)
1958+
fix!(H, [1 0; 0 1])
19571959
@test_throws(
19581960
ErrorException(
1959-
"either `x` or `A` must be constant in `quadform(x, A)`",
1961+
"Convex.jl v0.13.5 introduced the ability to use `fix!`ed variables " *
1962+
"in `quadform`. However, this did not consider the case that the " *
1963+
"value of `fix!`ed variables is changed between solves. Due to the " *
1964+
"risk that this may silently produce incorrect solutions, this " *
1965+
"behavior has been removed. Use `evaluate(H)` to obtain the value of " *
1966+
"a fixed variable. If the value changes between solves, rebuild the " *
1967+
"problem for the change to take effect.",
19601968
),
1961-
quadform(Variable(2), Variable(2, 2)),
1969+
quadform(Variable(2), H),
19621970
)
19631971
@test_throws(
19641972
ErrorException("quadform only takes square matrices"),

0 commit comments

Comments
 (0)