Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for {Hermitian,Symmetric} in Zeros constraint #3281

Merged
merged 7 commits into from
Mar 15, 2023
Merged

Add support for {Hermitian,Symmetric} in Zeros constraint #3281

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
eb171ef
Add support for {Hermitian,Symmetric} in Zeros constraint
odow Mar 12, 2023
c6d3707
Fix formattinng
odow Mar 12, 2023
2332a57
Expand errors
odow Mar 13, 2023
c5f21d8
Fixes
odow Mar 14, 2023
f5ae40d
More updates
odow Mar 14, 2023
839459b
Fix docs
odow Mar 15, 2023
c7bbaba
Update constraints.md
odow Mar 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions src/sd.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,3 +629,46 @@ function build_constraint(
shape,
)
end

function build_constraint(
_error::Function,
H::LinearAlgebra.Hermitian,
::Zeros,
)
n = LinearAlgebra.checksquare(H)
shape = HermitianMatrixShape(n)
x = vectorize(H, shape)
return build_constraint(error, x, MOI.Zeros(length(x)))
odow marked this conversation as resolved.
Show resolved Hide resolved
end

function build_constraint(
_error::Function,
f::LinearAlgebra.Symmetric,
::Zeros,
)
n = LinearAlgebra.checksquare(f)
shape = SymmetricMatrixShape(n)
x = vectorize(f, shape)
return build_constraint(error, x, MOI.Zeros(length(x)))
odow marked this conversation as resolved.
Show resolved Hide resolved
end

function build_constraint(_error::Function, ::AbstractMatrix, ::Nonnegatives)
return _error(
"Unsupported matrix in vector-valued set. Did you mean to use the " *
"broadcasting syntax `.>=` instead?",
odow marked this conversation as resolved.
Show resolved Hide resolved
)
end

function build_constraint(_error::Function, ::AbstractMatrix, ::Nonpositives)
return _error(
"Unsupported matrix in vector-valued set. Did you mean to use the " *
"broadcasting syntax `.<=` instead?",
)
end

function build_constraint(_error::Function, ::AbstractMatrix, ::Zeros)
return _error(
"Unsupported matrix in vector-valued set. Did you mean to use the " *
"broadcasting syntax `.==` instead?",
)
end
25 changes: 25 additions & 0 deletions test/test_constraint.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1544,4 +1544,29 @@ function test_extension_Zeros(ModelType = Model, VariableRefType = VariableRef)
return
end

function test_hermitian_in_zeros()
model = Model()
@variable(model, H[1:2, 1:2] in HermitianPSDCone())
c = @constraint(model, H == LinearAlgebra.I)
con = constraint_object(c)
@test isequal_canonical(con.func[1], real(H[1, 1]) - 1)
@test isequal_canonical(con.func[2], real(H[1, 2]))
@test isequal_canonical(con.func[3], real(H[2, 2]) - 1)
@test isequal_canonical(con.func[4], imag(H[1, 2]))
@test con.set == MOI.Zeros(4)
odow marked this conversation as resolved.
Show resolved Hide resolved
return
end

function test_symmetric_in_zeros()
model = Model()
@variable(model, H[1:2, 1:2], Symmetric)
c = @constraint(model, H == LinearAlgebra.I)
con = constraint_object(c)
@test isequal_canonical(con.func[1], H[1, 1] - 1)
@test isequal_canonical(con.func[2], H[1, 2] - 0)
@test isequal_canonical(con.func[3], H[2, 2] - 1)
@test con.set == MOI.Zeros(3)
odow marked this conversation as resolved.
Show resolved Hide resolved
return
end

end
31 changes: 31 additions & 0 deletions test/test_macros.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1894,4 +1894,35 @@ end

#!format: on

function test_matrix_in_vector_set()
model = Model()
@variable(model, X[1:2, 1:2])
A = [1 2; 3 4]
@test_throws_strip(
ErrorException(
"In `@constraint(model, X >= A)`: Unsupported matrix in " *
"vector-valued set. Did you mean to use the broadcasting syntax " *
"`.>=` instead?",
),
@constraint(model, X >= A),
)
@test_throws_strip(
ErrorException(
"In `@constraint(model, X <= A)`: Unsupported matrix in " *
"vector-valued set. Did you mean to use the broadcasting syntax " *
"`.<=` instead?",
),
@constraint(model, X <= A),
)
@test_throws_strip(
ErrorException(
"In `@constraint(model, X == A)`: Unsupported matrix in " *
"vector-valued set. Did you mean to use the broadcasting syntax " *
"`.==` instead?",
),
@constraint(model, X == A),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We recently added (#3273) support for vector == vector. But it is less clear what to do for matrix == matrix.

It gets lowered to X - A in Zeros(), but X - A is a matrix, and Zeros() is a vector-valued set, which triggers this error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make the equality case work.
The inequality, it's better that it's an error since the user most probably forgot PSDCone()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it wouldn't even be possible to support inequalities, as JuMP cannot know whether the user wants PSDCone() or HermitianPSDCone().

In any case, I concur, allowing inequalities leads to ambiguities. Maybe the user has a symmetric matrix, but wants to constrain it to be elementwise nonnegative.

There's also a bug that always happens with YALMIP: the user has a matrix they think is Hermitian but it's actually not because of numerical noise. Then the user writes >= 0 thinking it's constraining it to be PSD, but YALMIP instead interprets that as elementwise non-negativity.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also a bug that always happens with YALMIP: the user has a matrix they think is Hermitian but it's actually not because of numerical noise. Then the user writes >= 0 thinking it's constraining it to be PSD, but YALMIP instead interprets that as elementwise non-negativity.

Indeed, we want to avoid that, an error asking the user to be explicit about it is better than trying to save a few keystrokes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge this and see how we go re-feedback. The non-symmetric case X .== Y is pretty easy to type, and there's a nice error message.

The specialized support for symmetric and hermitian is more useful because of the redundant constraints.

Agree on inequalities.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The case of equalities is not so clear-cut because the user intention is not ambiguous. If JuMP interprets == as .== in the non-symmetric case the result will be a slowdown, not a bug (assuming the user thought the matrix was Hermitian).

I would just change the error message to say "Please declare your matrix as Symmetric or Hermitian if it is the case. Otherwise use .=="

We can't really do that since JuMP is a general purpose solver so having equality between matrices is completely fine and it's weird to talk about Symmetric or Hermitian matrices outside the context of SDP.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, but the error message seems to imply that == is never supported for matrices, which is not the case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, JuMP.Zeros is indeed not a vector-valued set (since it works with scalars, Symmetric and Hermitian). "Equality between $(typeof(matrix)). Use .== for elementwise inequality or wrap the matrix in Symmetric or Hermitian) for equality between matrices of such special structure"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thanks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the docs and the error message.

)
return
end

end # module